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_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]