diff --git a/docs/semantic-cleaning.md b/docs/semantic-cleaning.md index 82ef69b..f007e09 100644 --- a/docs/semantic-cleaning.md +++ b/docs/semantic-cleaning.md @@ -129,7 +129,9 @@ next_cleaned, next_report = fd.clean( `model_id="semantic::memory"` mark which decisions came from memory, and `Action.metadata` carries the raw/proposed value and evidence either way. - Retrieval matches on the exact normalized value first, falling back to a lightweight, - no-dependency similarity check (`difflib`) for minor value drift; low-similarity or + no-dependency similarity check (`difflib`) for minor value drift. Only exact normalized + matches can be auto-applied: a similarity match is capped below + `semantic_auto_threshold` and is only suggested for review, and low-similarity or conflicting repairs are never auto-applied. ## Configuration diff --git a/src/freshdata/semantic/canonical.py b/src/freshdata/semantic/canonical.py index 791050f..37314ff 100644 --- a/src/freshdata/semantic/canonical.py +++ b/src/freshdata/semantic/canonical.py @@ -19,6 +19,7 @@ from __future__ import annotations +import math import re import unicodedata @@ -245,6 +246,23 @@ def _render(payload: str, shape: str) -> str | None: return "".join(out) +def _group_lengths(value: str) -> list[int]: + """Lengths of the separator-delimited alphanumeric groups of *value* + (``"555 0101"`` -> ``[3, 4]``; ``"1.10"`` -> ``[1, 2]``).""" + lengths: list[int] = [] + run = 0 + for ch in value: + if ch in _SAFE_SEPARATORS: + if run: + lengths.append(run) + run = 0 + else: + run += 1 + if run: + lengths.append(run) + return lengths + + class ShapeAlignmentExpert: """Align separator drift to a column's dominant value template. @@ -253,6 +271,13 @@ class ShapeAlignmentExpert: re-rendered into the dominant template (``"555 0101"`` -> ``"555-0101"``). The payload is untouched by construction, which is what lets the policy gate admit these repairs even in identifier-like columns. + + Where the separators sit carries meaning too (``"1.10"`` and ``"1.1.0"`` + are different versions), so digits are never re-split across groups: a + value that already has separator-delimited groups is aligned only when its + groups map one-to-one onto the template's groups (same count, same + lengths). A value with no separators at all could be split several ways, + so its alignment is only suggested for review. """ name = "shape_alignment" @@ -295,6 +320,7 @@ def propose(self, series: pd.Series, info: SemanticColumnInfo) -> list[SemanticP # such candidates are held for human review instead of applied. distinct_ratio = (info.nunique or 0) / max(info.n_nonnull or 1, 1) existing = {value for value in counts.index if isinstance(value, str)} + dominant_groups = _group_lengths(dominant) out: list[SemanticProposal] = [] for raw, count in counts.items(): if _shape(raw) == dominant: @@ -302,7 +328,42 @@ def propose(self, series: pd.Series, info: SemanticColumnInfo) -> list[SemanticP rendered = _render(_payload(raw), dominant) if rendered is None or rendered == raw: continue + raw_groups = _group_lengths(raw) + # An unseparated value re-split into several groups is a guess + # about where the boundaries go; review it instead of applying. + resplit = len(raw_groups) == 1 and len(dominant_groups) > 1 + if not resplit and raw_groups != dominant_groups: + continue # regrouping digits would change the value's meaning collides = rendered in existing and distinct_ratio >= 0.5 + if collides: + detail = ( + "aligning would duplicate an existing value in a " + "mostly-distinct column; possible key collision" + ) + rationale = ( + "realigning the separators would make this value " + "identical to another existing value in a " + "mostly-distinct column; a possible key collision " + "needs human review" + ) + elif resplit: + detail = ( + "the value has no separators; splitting it into the " + "dominant template's groups is not provably meaning-preserving" + ) + rationale = ( + "inserting separators would decide where the value's " + "groups begin and end; the split needs human review" + ) + else: + detail = ( + "the value's separator-delimited groups map one-to-one onto " + "the dominant template; only separators change" + ) + rationale = ( + "separators realigned to the column's dominant " + "format; the alphanumeric payload is unchanged" + ) evidence = ( SemanticEvidence( "value_share", @@ -310,18 +371,9 @@ def propose(self, series: pd.Series, info: SemanticColumnInfo) -> list[SemanticP "column's values", 0.0, ), - SemanticEvidence( - "pattern", - ( - "aligning would duplicate an existing value in a " - "mostly-distinct column; possible key collision" - if collides - else "the value's alphanumeric payload fits the " - "dominant template exactly; only separators change" - ), - 0.0, - ), + SemanticEvidence("pattern", detail, 0.0), ) + needs_review = collides or resplit out.append( make_proposal( column=info.name, @@ -329,22 +381,10 @@ def propose(self, series: pd.Series, info: SemanticColumnInfo) -> list[SemanticP proposed_value=rendered, issue_type=self.issue_type, expert=self.name, - base_confidence=0.75 if collides else 0.96, + base_confidence=0.75 if needs_review else 0.96, evidence=evidence, count=int(count), - rationale=( - ( - "realigning the separators would make this value " - "identical to another existing value in a " - "mostly-distinct column; a possible key collision " - "needs human review" - ) - if collides - else ( - "separators realigned to the column's dominant " - "format; the alphanumeric payload is unchanged" - ) - ), + rationale=rationale, info=info, risk_override="high" if collides else None, ) @@ -358,7 +398,38 @@ def propose(self, series: pd.Series, info: SemanticColumnInfo) -> list[SemanticP _PERCENT_VALUE = re.compile(r"^\s*[+-]?\d+(?:\.\d+)?\s*%\s*$") _PERCENT_NAME = re.compile(r"percent|pct|rate|ratio", re.I) +#: Names that say "percent" outright; ``rate``/``ratio`` columns are just as +#: often fractions on a 0-1 scale. +_STRONG_PERCENT_NAME = re.compile(r"percent|pct", re.I) _EURO_GROUPED = re.compile(r"^\s*[+-]?\d{1,3}(?:\.\d{3})+,\d{1,2}\s*$") +#: Minimum plain numeric observations needed to infer a column's scale. +_SCALE_EVIDENCE_MIN = 3 + + +def _plain_numbers(counts: pd.Series) -> tuple[int, bool]: + """``(observations, all_in_unit_interval)`` over the column's plain + numeric values (``'%'``-suffixed strings and non-numbers excluded).""" + observations = 0 + unit_interval = True + for raw, count in counts.items(): + if isinstance(raw, bool): + continue + if isinstance(raw, str): + if "%" in raw: + continue + try: + number = float(raw.strip()) + except ValueError: + continue + elif isinstance(raw, (int, float)): + number = float(raw) + else: + continue + if math.isnan(number): + continue + observations += int(count) + unit_interval = unit_interval and 0.0 <= number <= 1.0 + return observations, unit_interval class NumericFormatExpert: @@ -368,6 +439,13 @@ class NumericFormatExpert: (the suffix is redundant with the column's meaning), and a European-grouped number (``"1.234,56"`` — dot thousands *and* comma decimal present, so the reading is unambiguous). Anything else is left to dtype repair. + + The percent case checks the column's scale against its other numeric + values. When they are all fractions in [0, 1] the column is not + percent-denominated, so ``"45%"`` is proposed as ``0.45``; it is held for + review because a percent straggler in a fraction column may also be an + entry error. A ``rate``/``ratio`` column with too few plain numbers to + show its scale is held for review as well. """ name = "numeric_format" @@ -382,17 +460,50 @@ def applies(self, info: SemanticColumnInfo) -> bool: def propose(self, series: pd.Series, info: SemanticColumnInfo) -> list[SemanticProposal]: out: list[SemanticProposal] = [] + counts = _value_counts(series) percent_column = bool(_PERCENT_NAME.search(info.name)) - for raw, count in _value_counts(series).items(): + strong_percent_name = bool(_STRONG_PERCENT_NAME.search(info.name)) + scale_known = False + fraction_scale = False + if percent_column: + observations, unit_interval = _plain_numbers(counts) + scale_known = observations >= _SCALE_EVIDENCE_MIN + fraction_scale = scale_known and unit_interval + for raw, count in counts.items(): if not isinstance(raw, str): continue + base_confidence = 0.96 if percent_column and _PERCENT_VALUE.match(raw): - value = float(raw.strip().rstrip("%").strip()) - rationale = ( - "the '%' suffix is redundant in a percent-denominated " - f"column; parses exactly to {value}" - ) - detail = "column name declares percent denomination" + number = float(raw.strip().rstrip("%").strip()) + if fraction_scale: + value = round(number / 100.0, 12) + base_confidence = 0.80 + rationale = ( + "the column's other values are fractions in [0, 1], so " + f"{raw.strip()!r} reads as {value}; held for review " + "because the scale is inferred from the data" + ) + detail = "column's plain numeric values all lie in [0, 1]" + elif not scale_known and not strong_percent_name: + value = number + base_confidence = 0.80 + rationale = ( + "a rate/ratio column may hold fractions or percents and " + "there are too few plain numbers to tell; parsing " + f"{raw.strip()!r} as {value} needs human review" + ) + detail = "column scale (fraction vs percent) is unknown" + else: + value = number + rationale = ( + "the '%' suffix is redundant in a percent-denominated " + f"column; parses exactly to {value}" + ) + detail = ( + "column's plain numeric values are on a percent scale" + if scale_known + else "column name declares percent denomination" + ) elif _EURO_GROUPED.match(raw): value = float(raw.strip().replace(".", "").replace(",", ".")) rationale = ( @@ -413,7 +524,7 @@ def propose(self, series: pd.Series, info: SemanticColumnInfo) -> list[SemanticP proposed_value=value, issue_type=self.issue_type, expert=self.name, - base_confidence=0.96, + base_confidence=base_confidence, evidence=evidence, count=int(count), rationale=rationale, diff --git a/src/freshdata/semantic/memory.py b/src/freshdata/semantic/memory.py index 5490eee..4cf234f 100644 --- a/src/freshdata/semantic/memory.py +++ b/src/freshdata/semantic/memory.py @@ -22,7 +22,7 @@ import pandas as pd -from .experts import VALUE_EXPERTS +from .experts import VALUE_EXPERTS, SemanticExpert from .scoring import make_proposal from .types import ( SemanticColumnInfo, @@ -220,7 +220,29 @@ def learn_semantic_repairs(decisions: Any) -> list[dict[str, Any]]: # Retrieval # --------------------------------------------------------------------------- # -_EXPERTS_BY_ISSUE = {e.issue_type: e for e in VALUE_EXPERTS} +#: Experts are keyed by their unique ``name``: several experts share one +#: ``issue_type`` (``format_alignment`` covers NFC, shape alignment, 24:00 and +#: ISO-instant repairs), so an issue-type map would keep only the last of them +#: and gate every learned repair with the wrong ``applies`` check. +_EXPERTS_BY_NAME: dict[str, SemanticExpert] = {e.name: e for e in VALUE_EXPERTS} +_EXPERTS_BY_ISSUE: dict[str, list[SemanticExpert]] = {} +for _expert in VALUE_EXPERTS: + _EXPERTS_BY_ISSUE.setdefault(_expert.issue_type, []).append(_expert) +del _expert + + +def _replay_expert(repair: dict[str, Any]) -> SemanticExpert | None: + """The expert whose ``applies`` check gates replay of *repair*. + + Looked up by the stored expert name. Records without a known name fall + back to the issue type only when exactly one expert handles it; an + ambiguous issue type is not replayed rather than gated by a guess. + """ + expert = _EXPERTS_BY_NAME.get(str(repair.get("expert") or "")) + if expert is not None: + return expert + candidates = _EXPERTS_BY_ISSUE.get(str(repair.get("issue_type") or ""), []) + return candidates[0] if len(candidates) == 1 else None def _reconstruct_value(stored: object, proposed_type: str) -> object: @@ -247,9 +269,11 @@ def semantic_memory_proposals( and a repair's column must still pass its expert's ``applies(info)`` check against the *current* data (not stale assumptions). Exact normalized-value matches keep the learned confidence; fuzzy matches (similarity >= 0.92) are - capped at ``min(learned_confidence, similarity)``, so drift never inflates - confidence. Risk is recomputed from that (possibly reduced) confidence by - the normal scoring path, same as any deterministic proposal. + capped at ``min(learned_confidence, similarity)`` *and* strictly below the + auto-apply threshold. A near-miss value is a different value — often an + already-valid one — so a fuzzy replay is only ever suggested for review, + never auto-applied. Risk is recomputed from that (possibly reduced) + confidence by the normal scoring path, same as any deterministic proposal. """ out = SemanticProposalSet() match = memory.match(df) @@ -267,7 +291,7 @@ def semantic_memory_proposals( if not column or column not in ctx.columns: continue info = ctx.columns[column] - expert = _EXPERTS_BY_ISSUE.get(issue_type) + expert = _replay_expert(repair) if expert is None or not expert.applies(info): continue # column is no longer eligible for this kind of repair @@ -279,6 +303,8 @@ def semantic_memory_proposals( learned_raw = repair.get("raw_value") learned_norm = _normalize_text(learned_raw) learned_confidence = float(repair.get("confidence", 0.0)) + # Fuzzy replays stay strictly below the auto threshold (review band). + fuzzy_ceiling = max(0.0, ctx.auto_threshold - 0.01) try: counts = df[column].value_counts(dropna=True) @@ -287,12 +313,14 @@ def semantic_memory_proposals( for raw, count in counts.items(): if not isinstance(raw, str): continue - similarity = _similarity(learned_norm, _normalize_text(raw)) + raw_norm = _normalize_text(raw) + similarity = _similarity(learned_norm, raw_norm) if similarity < SIMILARITY_THRESHOLD: continue - retrieved_confidence = ( - learned_confidence if similarity >= 0.999 else min(learned_confidence, similarity) - ) + if raw_norm == learned_norm: + retrieved_confidence = learned_confidence + else: + retrieved_confidence = min(learned_confidence, similarity, fuzzy_ceiling) evidence = ( SemanticEvidence( "memory_replay", diff --git a/tests/test_semantic_repair_safety.py b/tests/test_semantic_repair_safety.py new file mode 100644 index 0000000..c6c91c4 --- /dev/null +++ b/tests/test_semantic_repair_safety.py @@ -0,0 +1,226 @@ +"""Regression tests for accuracy-sensitive semantic repairs. + +- #252: a fuzzy memory match must never auto-apply to a different value. +- #253: ``'45%'`` in a rate column of fractions is not rewritten as ``45.0``. +- #254: shape alignment never re-splits digits across groups (``'1.10'``). +- #300: memory replay gates each learned repair by its own expert. +""" + +from __future__ import annotations + +import pandas as pd +import pytest + +import freshdata as fd +from freshdata.config import CleanConfig +from freshdata.semantic.canonical import NumericFormatExpert, ShapeAlignmentExpert +from freshdata.semantic.context import build_semantic_context +from freshdata.semantic.experts import VALUE_EXPERTS +from freshdata.semantic.memory import _replay_expert, semantic_memory_proposals + + +def _semantic(report: fd.CleanReport) -> list[fd.Action]: + return [a for a in report.actions if a.step == "semantic"] + + +def _clean(df: pd.DataFrame, **kwargs): + return fd.clean(df, semantic_mode="auto", return_report=True, verbose=False, **kwargs) + + +def _proposals(expert, df: pd.DataFrame): + column = df.columns[0] + ctx = build_semantic_context(df, CleanConfig(semantic_mode="auto")) + info = ctx.columns[column] + assert expert.applies(info) + return expert.propose(df[column], info) + + +# --------------------------------------------------------------------------- # +# #252: fuzzy memory replay is review-only +# --------------------------------------------------------------------------- # + +_LEARN_EMAILS = ["alice.johnson@EXAMPLE.COM", "bob@x.org", "carl@y.net", "dan@z.io"] +#: One character away from the learned raw value, and already a valid email. +_DRIFTED_EMAILS = ["alice.johnsen@example.com", "bob@x.org", "carl@y.net", "dan@z.io"] + + +@pytest.fixture +def email_memory(): + df = pd.DataFrame({"email": _LEARN_EMAILS}) + _, report = _clean(df) + return fd.learn_cleaning_memory(df, decisions=report, dataset_id="crm") + + +def test_fuzzy_memory_match_does_not_rewrite_a_different_valid_value(email_memory) -> None: + df = pd.DataFrame({"email": _DRIFTED_EMAILS}) + out, report = _clean(df, memory=email_memory) + + assert out["email"][0] == "alice.johnsen@example.com" + replayed = [a for a in _semantic(report) if a.memory_influenced] + assert replayed, "the fuzzy match should still be surfaced as a suggestion" + assert all(a.status == "suggested" and a.human_review for a in replayed) + assert all(a.confidence < 0.95 for a in replayed) + + +def test_fuzzy_memory_cap_follows_the_configured_auto_threshold(email_memory) -> None: + df = pd.DataFrame({"email": _DRIFTED_EMAILS}) + ctx = build_semantic_context( + df, CleanConfig(semantic_mode="auto", semantic_auto_threshold=0.9) + ) + proposals = list(semantic_memory_proposals(df, ctx, email_memory)) + assert proposals + assert all(p.confidence < 0.9 for p in proposals) + + +def test_exact_memory_match_still_auto_applies(email_memory) -> None: + out, report = _clean(pd.DataFrame({"email": _LEARN_EMAILS}), memory=email_memory) + + assert out["email"][0] == "alice.johnson@example.com" + replayed = [a for a in _semantic(report) if a.memory_influenced] + assert replayed and all(a.status == "automatic" for a in replayed) + + +# --------------------------------------------------------------------------- # +# #253: percent stragglers respect the column's scale +# --------------------------------------------------------------------------- # + + +def test_percent_in_fraction_rate_column_is_not_auto_applied_as_45() -> None: + df = pd.DataFrame({"conversion_rate": ["0.12", "0.30", "0.25", "45%", "0.5", "0.41"]}) + out, report = _clean(df) + + assert 45.0 not in pd.to_numeric(out["conversion_rate"], errors="coerce").tolist() + actions = [a for a in _semantic(report) if a.metadata.get("raw_value") == "45%"] + assert len(actions) == 1 + action = actions[0] + assert action.status == "suggested" and action.human_review + assert action.metadata["proposed_value"] == pytest.approx(0.45) + + +def test_percent_in_fraction_column_proposes_value_over_100() -> None: + df = pd.DataFrame({"win_ratio": ["0.1", "0.2", "0.9", "30%"]}) + (proposal,) = _proposals(NumericFormatExpert(), df) + assert proposal.proposed_value == pytest.approx(0.30) + assert proposal.confidence < 0.95 + + +def test_rate_column_without_scale_evidence_is_held_for_review() -> None: + # Only two plain numbers: too few to tell fractions from percents. + df = pd.DataFrame({"growth_rate": ["12", "30", "45%"]}) + (proposal,) = _proposals(NumericFormatExpert(), df) + assert proposal.proposed_value == 45.0 + assert proposal.confidence < 0.95 + + +def test_percent_scale_rate_column_still_auto_applies() -> None: + df = pd.DataFrame({"tax_rate": ["12", "30", "25", "45%", "50", "41"]}) + out, report = _clean(df) + + assert out["tax_rate"].tolist() == [12.0, 30.0, 25.0, 45.0, 50.0, 41.0] + actions = [a for a in _semantic(report) if a.metadata.get("raw_value") == "45%"] + assert actions and all(a.status == "automatic" for a in actions) + + +def test_strong_percent_name_keeps_the_percent_number() -> None: + # TruthBench edu-07 shape: score_percent on a 0-100 scale. + df = pd.DataFrame({"score_percent": ["95", "90", "87.5", "82", "95%"]}) + out, report = _clean(df) + + assert out["score_percent"].tolist()[-1] == 95.0 + actions = [a for a in _semantic(report) if a.metadata.get("raw_value") == "95%"] + assert actions and all(a.status == "automatic" for a in actions) + + +# --------------------------------------------------------------------------- # +# #254: shape alignment never regroups digits +# --------------------------------------------------------------------------- # + + +def test_version_with_different_grouping_is_left_unchanged() -> None: + values = ["1.2.3", "2.0.1", "3.4.5", "1.0.0", "2.2.2", "4.1.0", "1.10"] + out, report = _clean(pd.DataFrame({"version": values})) + + assert out["version"].tolist() == values + assert not [a for a in _semantic(report) if a.metadata.get("raw_value") == "1.10"] + + +@pytest.mark.parametrize("raw", ["1.10", "12.3", "1.1.0.0"]) +def test_regrouping_candidates_are_not_proposed(raw: str) -> None: + df = pd.DataFrame({"version": ["1.2.3", "2.0.1", "3.4.5", "1.0.0", "2.2.2", "4.1.0", raw]}) + proposals = _proposals(ShapeAlignmentExpert(), df) + assert [p for p in proposals if p.raw_value == raw] == [] + + +def test_separator_drift_with_matching_groups_still_auto_applies() -> None: + # TruthBench crm-04 shape: "555 0101" among "555-0101". + values = ["555-0101", "555-0102", "555-0103", "555-0104", "555-0105", "555 0106"] + out, report = _clean(pd.DataFrame({"code": values})) + + assert out["code"].tolist()[-1] == "555-0106" + actions = [a for a in _semantic(report) if a.metadata.get("raw_value") == "555 0106"] + assert actions and all(a.status == "automatic" for a in actions) + + +def test_unseparated_value_split_into_template_groups_is_review_only() -> None: + df = pd.DataFrame({"code": ["555-0101", "555-0102", "555-0103", "555-0104", "5550105"]}) + out, report = _clean(df) + + assert out["code"].tolist()[-1] == "5550105" + (proposal,) = [ + p for p in _proposals(ShapeAlignmentExpert(), df) if p.raw_value == "5550105" + ] + assert proposal.proposed_value == "555-0105" + assert proposal.confidence < 0.95 + + +# --------------------------------------------------------------------------- # +# #300: replay is gated by the repair's own expert +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize("expert", VALUE_EXPERTS, ids=lambda e: e.name) +def test_replay_expert_resolves_by_stored_expert_name(expert) -> None: + assert _replay_expert({"expert": expert.name, "issue_type": expert.issue_type}) is expert + + +def test_replay_expert_falls_back_to_issue_type_only_when_unambiguous() -> None: + by_issue: dict[str, list] = {} + for expert in VALUE_EXPERTS: + by_issue.setdefault(expert.issue_type, []).append(expert) + for issue_type, experts in by_issue.items(): + resolved = _replay_expert({"expert": None, "issue_type": issue_type}) + if len(experts) == 1: + assert resolved is experts[0] + else: + assert resolved is None + assert _replay_expert({"expert": "no_such_expert", "issue_type": "format_alignment"}) is None + + +_REPLAY_FRAMES = { + "unicode_nfc": {"city": ["Jose\u0301", "Lima", "Quito", "Bogota"]}, + "mojibake": {"venue": ["Café", "Bar", "Pub", "Inn"]}, + "shape_alignment": { + "code": ["555-0101", "555-0102", "555-0103", "555-0104", "555-0105", "555 0106"] + }, + "numeric_format": {"score_percent": ["95", "90", "87.5", "82", "95%"]}, + "time_canonical": {"start_time": ["09:00", "10:30", "11:15", "12:00", "24:00"]}, +} + + +@pytest.mark.parametrize("expert_name", sorted(_REPLAY_FRAMES)) +def test_learned_repair_replays_on_the_identical_frame(expert_name: str) -> None: + data = _REPLAY_FRAMES[expert_name] + df = pd.DataFrame(data) + _, report = _clean(df) + memory = fd.learn_cleaning_memory(df, decisions=report, dataset_id="d") + learned = memory.value_patterns["semantic_repairs"] + assert [r["expert"] for r in learned] == [expert_name] + + ctx = build_semantic_context(df, CleanConfig(semantic_mode="auto")) + assert len(semantic_memory_proposals(df, ctx, memory)) == 1 + + _, replay_report = _clean(pd.DataFrame(data), memory=memory) + actions = _semantic(replay_report) + assert actions + assert all(a.memory_influenced for a in actions) + assert all(a.metadata.get("expert") == expert_name for a in actions)