From d35f617b11582f56bc52688b9fdc7a34d72001a6 Mon Sep 17 00:00:00 2001 From: Kevin Turcios Date: Sun, 18 Oct 2026 06:08:10 -0500 Subject: [PATCH 1/2] fix: make SQLite term normalization idempotent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _prepare_term applied NFC before lowercasing. Lowercasing can produce a sequence that NFC composes differently (e.g. "T̈" -> "ẗ" -> "ẗ"), so normalizing a stored term again gave a different key: terms from get_terms() could not be looked up, and deserialize, which re-normalizes stored terms, silently lost them. Lowercase first, then apply NFC. Co-authored-by: wtfyd-agent[bot] <328989958+wtfyd-agent[bot]@users.noreply.github.com> --- .../storage/sqlite/semantic_ref_index.py | 10 ++++--- tests/test_sqlite_indexes.py | 28 +++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/typeagent/storage/sqlite/semantic_ref_index.py b/src/typeagent/storage/sqlite/semantic_ref_index.py index 125a374a..0b143e7e 100644 --- a/src/typeagent/storage/sqlite/semantic_ref_index.py +++ b/src/typeagent/storage/sqlite/semantic_ref_index.py @@ -173,11 +173,13 @@ def _prepare_term(self, term: str) -> str: # Strip leading/trailing whitespace term = term.strip() + # Convert to lowercase before NFC: lowercasing can produce a sequence + # that composes differently (e.g. "T̈" -> "ẗ" -> "ẗ"), so the + # reverse order isn't idempotent and stored terms couldn't be looked up. + term = term.lower() + # 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 re.sub(r"\s+", " ", term) diff --git a/tests/test_sqlite_indexes.py b/tests/test_sqlite_indexes.py index c7c9af01..e96d9f1f 100644 --- a/tests/test_sqlite_indexes.py +++ b/tests/test_sqlite_indexes.py @@ -906,6 +906,34 @@ async def test_term_normalization_unicode(self, sqlite_db: sqlite3.Connection): assert len(results) == 1, f"Should roundtrip higher plane Unicode: '{term}'" assert results[0].semantic_ref_ordinal == 200 + i + @pytest.mark.asyncio + @pytest.mark.parametrize( + "term", + [ + "T̈", # Uppercase base + combining mark: lowercase composes to ẗ + "J̌", # ǰ has no uppercase precomposed form + "Α͂", # Greek polytonic: Α + perispomeni lowercases to ᾶ + ], + ) + async def test_term_normalization_is_idempotent( + self, sqlite_db: sqlite3.Connection, term: str + ): + """A normalized term must normalize to itself, or stored terms get lost.""" + index = SqliteTermToSemanticRefIndex(sqlite_db) + await index.add_term(term, 1) + + [stored] = await index.get_terms() + assert await index.lookup_term(stored) == await index.lookup_term(term) + assert await index.lookup_term(term.lower()) == await index.lookup_term(term) + + # deserialize re-normalizes the already-normalized stored terms. + data = await index.serialize() + await index.clear() + await index.deserialize(data) + results = await index.lookup_term(term) + assert results is not None + assert [r.semantic_ref_ordinal for r in results] == [1] + @pytest.mark.asyncio async def test_term_case_sensitivity(self, sqlite_db: sqlite3.Connection): """Test case normalization in _prepare_term().""" From ad57e207ea12b1f08665fcbcbaba87f50efe41f7 Mon Sep 17 00:00:00 2001 From: Kevin Turcios Date: Sun, 18 Oct 2026 16:51:39 -0500 Subject: [PATCH 2/2] fix: apply the idempotent ordering in the shared normalize_term Lowercase before NFC in typeagent.knowpro.common.normalize_term, which both backends now share (#325), instead of in the SQLite-only _prepare_term. Move the regression tests to tests/test_term_normalization.py, cover memory and SQLite, and note the stored-term compatibility impact in the CHANGELOG. --- CHANGELOG.md | 6 +++++ src/typeagent/knowpro/common.py | 13 +++++++---- tests/test_sqlite_indexes.py | 28 ----------------------- tests/test_term_normalization.py | 39 ++++++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ebf9d0f..78da8414 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,12 @@ 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. +- Term normalization now lowercases before applying NFC, so normalizing an + already-normalized term no longer changes it. This affects only terms where + lowercasing yields a sequence that composes differently (e.g. an uppercase + 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. ### 0.5.0 (May 26) diff --git a/src/typeagent/knowpro/common.py b/src/typeagent/knowpro/common.py index 0eebfc48..7f85788d 100644 --- a/src/typeagent/knowpro/common.py +++ b/src/typeagent/knowpro/common.py @@ -10,12 +10,17 @@ 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 + Strips surrounding whitespace, lowercases, applies NFC normalization, and + collapses runs of whitespace to a single space. Every term index must apply this on both write and lookup so backends agree on term identity. + + Lowercasing comes before NFC because lowercasing can produce a sequence + that composes differently (e.g. "T\u0308" -> "t\u0308" -> "\u1e97"); in the + reverse order the result isn't idempotent, so re-normalizing a stored term + (as deserialization does) would change it. """ - term = unicodedata.normalize("NFC", term.strip()) - return re.sub(r"\s+", " ", term).lower() + term = unicodedata.normalize("NFC", term.strip().lower()) + return re.sub(r"\s+", " ", term) def is_search_term_wildcard(search_term: SearchTerm) -> bool: diff --git a/tests/test_sqlite_indexes.py b/tests/test_sqlite_indexes.py index e96d9f1f..c7c9af01 100644 --- a/tests/test_sqlite_indexes.py +++ b/tests/test_sqlite_indexes.py @@ -906,34 +906,6 @@ async def test_term_normalization_unicode(self, sqlite_db: sqlite3.Connection): assert len(results) == 1, f"Should roundtrip higher plane Unicode: '{term}'" assert results[0].semantic_ref_ordinal == 200 + i - @pytest.mark.asyncio - @pytest.mark.parametrize( - "term", - [ - "T̈", # Uppercase base + combining mark: lowercase composes to ẗ - "J̌", # ǰ has no uppercase precomposed form - "Α͂", # Greek polytonic: Α + perispomeni lowercases to ᾶ - ], - ) - async def test_term_normalization_is_idempotent( - self, sqlite_db: sqlite3.Connection, term: str - ): - """A normalized term must normalize to itself, or stored terms get lost.""" - index = SqliteTermToSemanticRefIndex(sqlite_db) - await index.add_term(term, 1) - - [stored] = await index.get_terms() - assert await index.lookup_term(stored) == await index.lookup_term(term) - assert await index.lookup_term(term.lower()) == await index.lookup_term(term) - - # deserialize re-normalizes the already-normalized stored terms. - data = await index.serialize() - await index.clear() - await index.deserialize(data) - results = await index.lookup_term(term) - assert results is not None - assert [r.semantic_ref_ordinal for r in results] == [1] - @pytest.mark.asyncio async def test_term_case_sensitivity(self, sqlite_db: sqlite3.Connection): """Test case normalization in _prepare_term().""" diff --git a/tests/test_term_normalization.py b/tests/test_term_normalization.py index 9dda2847..cfb2c6d7 100644 --- a/tests/test_term_normalization.py +++ b/tests/test_term_normalization.py @@ -35,6 +35,22 @@ def test_normalize_term(raw: str, expected: str) -> None: assert normalize_term(raw) == expected +# Terms whose lowercase form composes differently under NFC, so normalizing +# NFC-then-lowercase isn't idempotent. +NON_IDEMPOTENT_TERMS = [ + "T\u0308", # Uppercase base + combining mark: lowercase composes to \u1e97 + "J\u030c", # \u01f0 has no uppercase precomposed form + "\u0391\u0342", # Greek polytonic: Alpha + perispomeni lowercases to \u1fb6 +] + + +@pytest.mark.parametrize("raw", NON_IDEMPOTENT_TERMS) +def test_normalize_term_is_idempotent(raw: str) -> None: + once = normalize_term(raw) + assert normalize_term(once) == once + assert normalize_term(raw.lower()) == once + + @pytest.fixture(params=["memory", "sqlite"]) def term_index( request: pytest.FixtureRequest, @@ -179,3 +195,26 @@ async def test_term_index_deserialize_dedupes_merged_ordinals( 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] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("term", NON_IDEMPOTENT_TERMS) +async def test_term_index_normalization_round_trips( + term_index: ITermToSemanticRefIndex, term: str +) -> None: + """A stored term must normalize to itself, or it can't be looked up.""" + await term_index.add_term(term, 1) + + [stored] = await term_index.get_terms() + found = await term_index.lookup_term(stored) + assert found is not None and [r.semantic_ref_ordinal for r in found] == [1] + found = await term_index.lookup_term(term.lower()) + assert found is not None and [r.semantic_ref_ordinal for r in found] == [1] + + # deserialize re-normalizes the already-normalized stored terms. + data = await term_index.serialize() + await term_index.clear() + await term_index.deserialize(data) + 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]