From cbbb7b28a39eaf24cfdbc569559c812bf173a945 Mon Sep 17 00:00:00 2001 From: Bernhard Merkle Date: Tue, 29 Sep 2026 00:27:04 +0200 Subject: [PATCH 1/3] Share term normalization between memory and SQLite indexes The memory term index only lowercased terms while the SQLite one also stripped, NFC-normalized and collapsed whitespace, so lookups, get_terms() and size() disagreed between backends. Add a shared normalize_term() and use it in both term indexes and both property indexes (name and value are normalized separately). Also merge, rather than overwrite, terms that collide after normalization in the memory deserialize, and use the shared function for the fuzzy related-terms lists. Fixes #322 Co-Authored-By: Claude Sonnet 5.5 --- src/typeagent/knowpro/add_messages.py | 3 +- src/typeagent/knowpro/common.py | 14 ++ src/typeagent/knowpro/conversation_base.py | 7 +- .../storage/memory/property_index.py | 15 ++- .../storage/memory/semantic_ref_index.py | 5 +- .../storage/sqlite/property_index.py | 27 ++-- .../storage/sqlite/semantic_ref_index.py | 16 +-- tests/test_term_normalization.py | 120 ++++++++++++++++++ 8 files changed, 163 insertions(+), 44 deletions(-) create mode 100644 tests/test_term_normalization.py diff --git a/src/typeagent/knowpro/add_messages.py b/src/typeagent/knowpro/add_messages.py index 9393bbf6..98de3074 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 semantic_ref_index +from .common import normalize_term from .interfaces import ( AddMessagesResult, IKnowledgeExtractor, @@ -250,7 +251,7 @@ def _collect_related_terms_for_fuzzy_index( 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 e8252dd9..5e8217f7 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 semantic_ref_index +from .common import normalize_term from .convsettings import ConversationSettings from .interfaces import ( AddMessagesResult, @@ -494,12 +495,12 @@ async def _update_related_terms_incremental( 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)) if new_terms: await fuzzy_index.add_terms(list(new_terms)) diff --git a/src/typeagent/storage/memory/property_index.py b/src/typeagent/storage/memory/property_index.py index ae928278..0abffd3d 100644 --- a/src/typeagent/storage/memory/property_index.py +++ b/src/typeagent/storage/memory/property_index.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,8 +269,7 @@ 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.""" @@ -315,7 +314,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/semantic_ref_index.py b/src/typeagent/storage/memory/semantic_ref_index.py index 17de695d..254a5d84 100644 --- a/src/typeagent/storage/memory/semantic_ref_index.py +++ b/src/typeagent/storage/memory/semantic_ref_index.py @@ -8,6 +8,7 @@ from typechat import Failure from ...knowpro import convknowledge, secondary_index +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/property_index.py b/src/typeagent/storage/sqlite/property_index.py index c4a10feb..3377275d 100644 --- a/src/typeagent/storage/sqlite/property_index.py +++ b/src/typeagent/storage/sqlite/property_index.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 property_index 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 = property_index.make_property_term_text(property_name, value) - term_text = term_text.lower() # Matches PropertyIndex._prepare_term_text - property_name, value = property_index.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 = property_index.make_property_term_text(property_name, value) - term_text = term_text.lower() - property_name, value = property_index.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 = property_index.make_property_term_text(property_name, value) - term_text = term_text.lower() # Matches PropertyIndex._prepare_term_text - property_name, value = property_index.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( diff --git a/src/typeagent/storage/sqlite/semantic_ref_index.py b/src/typeagent/storage/sqlite/semantic_ref_index.py index 125a374a..b0cb5334 100644 --- a/src/typeagent/storage/sqlite/semantic_ref_index.py +++ b/src/typeagent/storage/sqlite/semantic_ref_index.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, @@ -169,15 +168,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..22faf402 --- /dev/null +++ b/tests/test_term_normalization.py @@ -0,0 +1,120 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Memory and SQLite term indexes must normalize terms identically (#322).""" + +import sqlite3 + +import pytest + +from typeagent.knowpro.common import normalize_term +from typeagent.knowpro.interfaces import ( + IPropertyToSemanticRefIndex, + ITermToSemanticRefIndex, +) +from typeagent.storage.memory.property_index import PropertyIndex +from typeagent.storage.memory.semantic_ref_index import TermToSemanticRefIndex +from typeagent.storage.sqlite.property_index import SqlitePropertyIndex +from typeagent.storage.sqlite.schema import init_db_schema +from typeagent.storage.sqlite.semantic_ref_index 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): + 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): + 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] From f3748a28b43f4c1c838096bf3fb9cda3d91fb980 Mon Sep 17 00:00:00 2001 From: Bernhard Merkle Date: Tue, 29 Sep 2026 00:35:14 +0200 Subject: [PATCH 2/3] Address review feedback on term normalization - Normalize prop_name in remove_property (memory and SQLite) - Keep the empty normalized term through SQLite deserialize - Don't embed empty normalized terms in the incremental fuzzy update - Update the _collect_related_terms_for_fuzzy_index docstring - Annotate the test fixtures; add tests for removal and empty-term round trip Co-Authored-By: Claude Sonnet 5.5 --- src/typeagent/knowpro/add_messages.py | 4 ++- src/typeagent/knowpro/conversation_base.py | 3 +- .../storage/memory/property_index.py | 1 + .../storage/sqlite/property_index.py | 2 +- .../storage/sqlite/semantic_ref_index.py | 2 +- tests/test_term_normalization.py | 30 +++++++++++++++++-- 6 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/typeagent/knowpro/add_messages.py b/src/typeagent/knowpro/add_messages.py index 98de3074..9e244928 100644 --- a/src/typeagent/knowpro/add_messages.py +++ b/src/typeagent/knowpro/add_messages.py @@ -245,7 +245,9 @@ 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] = [] diff --git a/src/typeagent/knowpro/conversation_base.py b/src/typeagent/knowpro/conversation_base.py index 5e8217f7..f7386ce8 100644 --- a/src/typeagent/knowpro/conversation_base.py +++ b/src/typeagent/knowpro/conversation_base.py @@ -491,7 +491,7 @@ 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): @@ -501,6 +501,7 @@ async def _update_related_terms_incremental( elif isinstance(knowledge, Action): for verb in knowledge.verbs: 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/property_index.py b/src/typeagent/storage/memory/property_index.py index 0abffd3d..26dba174 100644 --- a/src/typeagent/storage/memory/property_index.py +++ b/src/typeagent/storage/memory/property_index.py @@ -274,6 +274,7 @@ async def lookup_property( 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) diff --git a/src/typeagent/storage/sqlite/property_index.py b/src/typeagent/storage/sqlite/property_index.py index 3377275d..96c3d8bb 100644 --- a/src/typeagent/storage/sqlite/property_index.py +++ b/src/typeagent/storage/sqlite/property_index.py @@ -121,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/semantic_ref_index.py b/src/typeagent/storage/sqlite/semantic_ref_index.py index b0cb5334..d541075e 100644 --- a/src/typeagent/storage/sqlite/semantic_ref_index.py +++ b/src/typeagent/storage/sqlite/semantic_ref_index.py @@ -148,7 +148,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): diff --git a/tests/test_term_normalization.py b/tests/test_term_normalization.py index 22faf402..b85047ed 100644 --- a/tests/test_term_normalization.py +++ b/tests/test_term_normalization.py @@ -3,6 +3,7 @@ """Memory and SQLite term indexes must normalize terms identically (#322).""" +from collections.abc import Generator import sqlite3 import pytest @@ -35,7 +36,9 @@ def test_normalize_term(raw: str, expected: str) -> None: @pytest.fixture(params=["memory", "sqlite"]) -def term_index(request: pytest.FixtureRequest): +def term_index( + request: pytest.FixtureRequest, +) -> Generator[ITermToSemanticRefIndex, None, None]: if request.param == "memory": yield TermToSemanticRefIndex() else: @@ -46,7 +49,9 @@ def term_index(request: pytest.FixtureRequest): @pytest.fixture(params=["memory", "sqlite"]) -def prop_index(request: pytest.FixtureRequest): +def prop_index( + request: pytest.FixtureRequest, +) -> Generator[IPropertyToSemanticRefIndex, None, None]: if request.param == "memory": yield PropertyIndex() else: @@ -118,3 +123,24 @@ async def test_property_index_normalizes_values( 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] From d17339ef02215907036bfb3b4e7acd11b349d9ab Mon Sep 17 00:00:00 2001 From: Bernhard Merkle Date: Sun, 11 Oct 2026 14:48:16 +0100 Subject: [PATCH 3/3] Skip empty terms and dedupe merged ordinals in term indexes - Normalize before the empty check in add_term/add_terms_batch so whitespace-only terms are not stored under an empty key. - Skip missing, None and empty terms on deserialize in both backends (the memory backend previously crashed on a missing term). - Dedupe semref ordinals when several raw terms normalize to the same key on deserialize, in memory and SQLite. - Remove the unused PropertyIndex._prepare_term_text. - Note in the CHANGELOG that SQLite property values from earlier versions may need re-ingesting. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 ++ .../storage/memory/property_index.py | 4 -- .../storage/memory/semantic_ref_index.py | 19 +++++-- .../storage/sqlite/semantic_ref_index.py | 13 +++-- tests/test_property_index.py | 9 ++- tests/test_term_normalization.py | 55 +++++++++++++++---- 6 files changed, 76 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf8148ea..6ebf9d0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,11 @@ Code importing the renamed `knowpro.interfaces_*` names from the `typeagent.knowpro.interfaces` aggregator is unaffected. +- Term and property indexes in memory and SQLite now share one normalization + (Unicode NFC, lowercase, trimmed, internal whitespace collapsed); whitespace-only + terms are no longer indexed (#322). SQLite property values stored by earlier + versions were only lowercased, so values with extra whitespace or decomposed + Unicode may no longer match; re-ingest such databases to rebuild the indexes. ### 0.5.0 (May 26) diff --git a/src/typeagent/storage/memory/property_index.py b/src/typeagent/storage/memory/property_index.py index 26dba174..b5db8b97 100644 --- a/src/typeagent/storage/memory/property_index.py +++ b/src/typeagent/storage/memory/property_index.py @@ -313,10 +313,6 @@ async def remove_all_for_semref(self, semref_id: int) -> None: for key in keys_to_remove: del self._map[key] - def _prepare_term_text(self, term_text: str) -> str: - """Do any pre-processing of the term.""" - 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( diff --git a/src/typeagent/storage/memory/semantic_ref_index.py b/src/typeagent/storage/memory/semantic_ref_index.py index 254a5d84..4cf45183 100644 --- a/src/typeagent/storage/memory/semantic_ref_index.py +++ b/src/typeagent/storage/memory/semantic_ref_index.py @@ -676,11 +676,11 @@ async def add_term( term: str, semantic_ref_ordinal: SemanticRefOrdinal | ScoredSemanticRefOrdinal, ) -> str: + term = self._prepare_term(term) if not term: return term if not isinstance(semantic_ref_ordinal, ScoredSemanticRefOrdinal): semantic_ref_ordinal = ScoredSemanticRefOrdinal(semantic_ref_ordinal, 1.0) - term = self._prepare_term(term) existing = self._map.get(term) if existing is not None: existing.append(semantic_ref_ordinal) @@ -731,12 +731,19 @@ async def deserialize(self, data: TermToSemanticRefIndexData) -> None: self._clear() for index_item_data in data["items"]: term = index_item_data.get("term") + if term is None: + continue term = self._prepare_term(term) - scored_refs_data = index_item_data["semanticRefOrdinals"] - scored_refs = [ - ScoredSemanticRefOrdinal.deserialize(s) for s in scored_refs_data - ] - self._map.setdefault(term, []).extend(scored_refs) + if not term: + continue + scored_refs = self._map.setdefault(term, []) + # Raw terms that normalize to the same key may share ordinals. + seen = {ref.semantic_ref_ordinal for ref in scored_refs} + for s in index_item_data["semanticRefOrdinals"]: + ref = ScoredSemanticRefOrdinal.deserialize(s) + if ref.semantic_ref_ordinal not in seen: + seen.add(ref.semantic_ref_ordinal) + scored_refs.append(ref) def _prepare_term(self, term: str) -> str: return normalize_term(term) diff --git a/src/typeagent/storage/sqlite/semantic_ref_index.py b/src/typeagent/storage/sqlite/semantic_ref_index.py index d541075e..68d90f90 100644 --- a/src/typeagent/storage/sqlite/semantic_ref_index.py +++ b/src/typeagent/storage/sqlite/semantic_ref_index.py @@ -46,11 +46,10 @@ async def add_term( term: str, semantic_ref_ordinal: SemanticRefOrdinal | ScoredSemanticRefOrdinal, ) -> str: + term = self._prepare_term(term) if not term: return term - term = self._prepare_term(term) - semref_id, score = _split_ordinal(semantic_ref_ordinal) cursor = self.db.cursor() @@ -72,9 +71,9 @@ async def add_terms_batch( return rows = [] for term, ordinal in terms: + term = self._prepare_term(term) if not term: continue - term = self._prepare_term(term) semref_id, score = _split_ordinal(ordinal) rows.append((term, semref_id, score)) if rows: @@ -147,9 +146,13 @@ async def deserialize(self, data: TermToSemanticRefIndexData) -> None: # Prepare all insertion data for bulk operation insertion_data = [] + # Raw terms that normalize to the same key may share ordinals. + seen: set[tuple[str, int]] = set() for item in data["items"]: if item and item.get("term") is not None: term = self._prepare_term(item["term"]) + if not term: + continue for semref_ordinal_data in item["semanticRefOrdinals"]: if isinstance(semref_ordinal_data, dict): semref_id = semref_ordinal_data["semanticRefOrdinal"] @@ -158,7 +161,9 @@ async def deserialize(self, data: TermToSemanticRefIndexData) -> None: # Fallback for direct integer semref_id = semref_ordinal_data score = 1.0 - insertion_data.append((term, semref_id, score)) + if (term, semref_id) not in seen: + seen.add((term, semref_id)) + insertion_data.append((term, semref_id, score)) # Bulk insert all the data if insertion_data: diff --git a/tests/test_property_index.py b/tests/test_property_index.py index 9a4e433c..51724b17 100644 --- a/tests/test_property_index.py +++ b/tests/test_property_index.py @@ -315,11 +315,10 @@ async def test_property_index_get_values(property_index: PropertyIndex): assert "value2" in values -def test_property_index_prepare_term_text(property_index: PropertyIndex): - """Test preprocessing term text in PropertyIndex.""" - term_text = "Prop.Name@@Value" - prepared_text = property_index._prepare_term_text(term_text) - assert prepared_text == "prop.name@@value" # Should be converted to lowercase +def test_property_index_make_term_text(property_index: PropertyIndex): + """Test building the normalized term text in PropertyIndex.""" + term_text = property_index._make_term_text(" Name", "Some Value") + assert term_text == "prop.name@@some value" @pytest.mark.asyncio diff --git a/tests/test_term_normalization.py b/tests/test_term_normalization.py index b85047ed..9dda2847 100644 --- a/tests/test_term_normalization.py +++ b/tests/test_term_normalization.py @@ -83,13 +83,14 @@ async def test_term_index_lookup_ignores_trailing_space( @pytest.mark.asyncio -async def test_term_index_whitespace_only_terms_agree( +async def test_term_index_ignores_whitespace_only_terms( term_index: ITermToSemanticRefIndex, ) -> None: - await term_index.add_term(" ", 0) + assert await term_index.add_term(" ", 0) == "" await term_index.add_term("\t", 0) + await term_index.add_terms_batch([("\n", 1), (" Foo", 2)]) assert await term_index.size() == 1 - assert await term_index.get_terms() == [""] + assert await term_index.get_terms() == ["foo"] @pytest.mark.asyncio @@ -135,12 +136,46 @@ async def test_property_index_remove_property_normalizes_name( @pytest.mark.asyncio -async def test_term_index_roundtrips_empty_normalized_term( +async def test_term_index_deserialize_skips_empty_and_missing_terms( 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] + data = { + "items": [ + { + "term": " ", + "semanticRefOrdinals": [{"semanticRefOrdinal": 1, "score": 1.0}], + }, + { + "term": None, + "semanticRefOrdinals": [{"semanticRefOrdinal": 2, "score": 1.0}], + }, + {"semanticRefOrdinals": [{"semanticRefOrdinal": 3, "score": 1.0}]}, + { + "term": "bar", + "semanticRefOrdinals": [{"semanticRefOrdinal": 4, "score": 1.0}], + }, + ] + } + await term_index.deserialize(data) # type: ignore[arg-type] + assert await term_index.get_terms() == ["bar"] + + +@pytest.mark.asyncio +async def test_term_index_deserialize_dedupes_merged_ordinals( + term_index: ITermToSemanticRefIndex, +) -> None: + data = { + "items": [ + { + "term": "Foo", + "semanticRefOrdinals": [{"semanticRefOrdinal": 1, "score": 1.0}], + }, + { + "term": "foo ", + "semanticRefOrdinals": [{"semanticRefOrdinal": 1, "score": 1.0}], + }, + ] + } + await term_index.deserialize(data) # type: ignore[arg-type] + found = await term_index.lookup_term("foo") + assert found is not None and [r.semantic_ref_ordinal for r in found] == [1]