fix(normalize_id): ensure idempotency and valid character output by adjusting casefolding order

graphify/ids.py

@@ -16,10 +16,19 @@ module exists so the recipe lives in one place and the two callers can no longer
 diverge.
 
 The recipe: NFKC-normalize (so composed/decomposed Unicode forms collapse),
-replace runs of non-word characters with a single underscore (``re.UNICODE`` so
-CJK/Cyrillic/Arabic/accented-Latin letters survive instead of collapsing to a
-per-file node), collapse repeated underscores, strip leading/trailing
-underscores, and casefold.
+casefold, NFKC-normalize again (casefold can *expand* a character into a base
+letter plus a combining mark — ``İ`` -> ``i`` + U+0307 — and the second pass
+recomposes what can be recomposed), replace runs of non-word characters with a
+single underscore (``re.UNICODE`` so CJK/Cyrillic/Arabic/accented-Latin letters
+survive instead of collapsing to a per-file node), collapse repeated
+underscores, then strip leading/trailing underscores.
+
+Casefolding runs BEFORE the non-word filter, not after. With it last, the
+combining marks casefold introduces were never filtered: ``İslemYap`` produced
+``i̇slemyap`` — an id containing U+0307, which is not a ``\\w`` character — and a
+second pass collapsed it to ``i_slemyap``, so the function was not idempotent
+and the builder's re-normalization disagreed with the extractor's ``make_id``
+for any Turkish identifier.
 """
 from __future__ import annotations
 
@@ -32,12 +41,21 @@ __all__ = ["normalize_id", "make_id"]
 def normalize_id(s: str) -> str:
     r"""Normalize a single ID string to its canonical form.
 
-    Idempotent: ``normalize_id(normalize_id(s)) == normalize_id(s)``.
+    Guarantees, both enforced by tests:
+
+    - Idempotent: ``normalize_id(normalize_id(s)) == normalize_id(s)``.
+    - The result contains only ``\w`` characters and ``_``.
+
+    Casefolding before the ``[^\w]+`` filter is what makes both hold — see the
+    module docstring for why the reverse order silently broke them.
     """
     s = unicodedata.normalize("NFKC", s)
+    # casefold can expand one character into a letter + combining mark, so it
+    # must run while the non-word filter can still see the result.
+    s = unicodedata.normalize("NFKC", s.casefold())
     s = re.sub(r"[^\w]+", "_", s, flags=re.UNICODE)
     s = re.sub(r"_+", "_", s)
-    return s.strip("_").casefold()
+    return s.strip("_")
 
 
 def make_id(*parts: str) -> str:

tests/test_id_normalization_contract.py

@@ -39,6 +39,30 @@ CONTRACT_CASES = [
     "x_c1",                       # must NOT be treated as a chunk suffix here
     "__dunder__",                 # leading/trailing underscores stripped
     "tab\tnewline\nspace ",       # whitespace runs -> single underscore
+    # Casefolding these EXPANDS them into a base letter plus a combining
+    # mark. With casefold last, the mark landed in the id after the [^\w] filter
+    # had already run, so the id carried a non-word character and a second pass
+    # changed it. Turkish identifiers are the common real-world case.
+    "İ",                     # İ  -> i + U+0307
+    "İslemYap",              # İslemYap
+    "fileİname",             # İ mid-identifier
+    "İı_Mixed",         # İ with dotless ı
+    "Große",                 # ß -> ss (length-changing casefold)
+    "ẞ",                     # ẞ capital sharp s -> ss
+]
+
+# Characters whose casefold expands or recomposes — the exact class that broke
+# the contract. Kept separate from CONTRACT_CASES because a few of them
+# (e.g. U+01F0) legitimately normalize to a precomposed character that is not
+# equal to its own casefold, which the lowercase assertion below would reject.
+CASE_EXPANDING_CHARS = [
+    "İ",  # İ  LATIN CAPITAL LETTER I WITH DOT ABOVE -> i + U+0307
+    "ǰ",  # ǰ  casefold expands, NFKC then recomposes it
+    "ͅ",  # ͅ   COMBINING GREEK YPOGEGRAMMENI -> ι
+    "ͺ",  # ͺ   GREEK YPOGEGRAMMENI -> space + ι
+    "ẞ",  # ẞ  -> ss
+    "ῗ",  # ῗ   iota with dialytika and perispomeni
+    "ὒ",  # ὒ   upsilon with psili and varia
 ]
 
 
@@ -84,6 +108,58 @@ def test_normalized_ids_are_safe_node_ids():
         assert not out.startswith("_") and not out.endswith("_")
 
 
+@pytest.mark.parametrize("ch", CASE_EXPANDING_CHARS)
+def test_case_expanding_chars_yield_word_only_ids(ch):
+    """The postcondition the old recipe silently broke.
+
+    ``normalize_id`` must emit only ``\\w`` characters and ``_``. Casefolding
+    last let the combining mark that ``İ``.casefold() produces slip past the
+    ``[^\\w]+`` filter, so ids carried U+0307 — invisible in most terminals, and
+    a second normalization pass then rewrote it to ``_``.
+    """
+    out = normalize_id(f"a{ch}b")
+    assert not re.search(r"[^\w]", out.replace("_", "")), (
+        f"normalize_id({ch!r}) -> {out!r} contains a non-word character "
+        f"({[hex(ord(c)) for c in out]})"
+    )
+
+
+@pytest.mark.parametrize("ch", CASE_EXPANDING_CHARS)
+def test_case_expanding_chars_are_idempotent(ch):
+    once = normalize_id(f"a{ch}b")
+    assert normalize_id(once) == once, (
+        f"normalize_id not idempotent for {ch!r}: {once!r} -> {normalize_id(once)!r}"
+    )
+
+
+@pytest.mark.parametrize("ch", CASE_EXPANDING_CHARS)
+def test_case_expanding_chars_normalize_case_insensitively(ch):
+    """The point of casefolding: upper and lower spellings must land on one id.
+
+    Asserted instead of ``out == out.casefold()`` because casefold and NFKC do
+    not commute — ``ǰ`` normalizes to the precomposed U+01F0, which is lowercase
+    but is not equal to its own casefold. Case-insensitivity is the property the
+    graph actually depends on.
+    """
+    assert normalize_id(f"a{ch.upper()}b") == normalize_id(f"a{ch.lower()}b")
+
+
+def test_turkish_identifier_ids_match_between_extractor_and_builder():
+    """End to end: the drift that split a Turkish symbol into ghost nodes.
+
+    ``make_id`` minted ``islem_i̇slemyap`` (with U+0307) while the builder's
+    re-normalization produced ``islem_i_slemyap``, so ``_semantic_id_remap``'s
+    ``startswith(new_stem)`` check missed and the re-key silently no-opped.
+    """
+    stem, symbol = "islem", "İslemYap"
+    minted = make_id(stem, symbol)
+    assert _normalize_id(minted) == minted, "builder re-normalization drifts from make_id"
+    assert minted.startswith(make_id(stem) + "_"), (
+        "symbol id lost its file stem prefix, so the re-key cannot relate them"
+    )
+    assert minted == "islem_i_slemyap"
+
+
 def test_both_callers_share_one_implementation():
     """Guard against re-forking: the two public callers must resolve to the same
     underlying function object as graphify.ids.normalize_id."""
@@ -117,3 +193,44 @@ def test_property_make_id_equals_normalize_id(s):
 def test_property_normalize_id_idempotent(s):
     once = normalize_id(s)
     assert normalize_id(once) == once
+
+
+# The plain st.text() property above already existed when this bug shipped, and did
+# not catch it: the bug needs one specific codepoint (U+0130) out of ~1.1M, which
+# a uniform draw essentially never produces. These strategies bias the search
+# toward the characters that actually stress the recipe — case-expanding letters
+# and combining marks — so the class stays covered rather than relying on luck.
+_stress_alphabet = st.one_of(
+    st.sampled_from(CASE_EXPANDING_CHARS),
+    st.sampled_from("Iıİi_.-/aZ0"),        # Turkish dotted/dotless pairs + separators
+    st.characters(categories=["Lu", "Ll", "Lt", "Mn", "Nd", "Pc"]),
+)
+_stress_text = st.text(alphabet=_stress_alphabet, max_size=12)
+
+
+@given(_stress_text)
+def test_property_normalize_id_idempotent_under_case_stress(s):
+    once = normalize_id(s)
+    assert normalize_id(once) == once, f"not idempotent for {s!r} -> {once!r}"
+
+
+@given(_stress_text)
+def test_property_normalize_id_emits_only_word_chars(s):
+    """The postcondition the old recipe violated: only \\w and _ may survive."""
+    out = normalize_id(s)
+    assert not re.search(r"[^\w]", out.replace("_", "")), (
+        f"normalize_id({s!r}) -> {out!r} leaked a non-word character"
+    )
+
+
+@given(_stress_text)
+def test_property_normalize_id_agrees_with_its_own_caseless_form(s):
+    """Feeding an already-caseless string must not change the answer.
+
+    Deliberately NOT ``normalize_id(s.upper()) == normalize_id(s.lower())``:
+    ``str.upper()`` is locale-independent and lossy, so Turkish dotless ``ı``
+    uppercases to ``I`` and then casefolds to ``i`` — the two spellings are
+    genuinely different characters, not a normalization failure. Caseless
+    equivalence via ``casefold`` is the invariant the graph relies on.
+    """
+    assert normalize_id(s) == normalize_id(s.casefold())