From a55109fce69afc3a397cd3dbb74f1ac86026cca3 Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 14:18:39 +0530 Subject: [PATCH] fix(context): quoted allowed values, dotted names, explicit confidence phrases, strict protection after schema resolution - split_value_list no longer splits on "/" and keeps quoted values whole. When a list has "," or ";", only those separate values; the last item may still split once on a trailing "and"/"or", and a leading "and"/"or" is dropped. Without commas, "and"/"or" still separate values ("dedupe by email and phone"). "Trinidad and Tobago, Chile" and "N/A, OK" now compile to the real values instead of fragments. The docstring and context docs say that ambiguous values need quoting. - The sentence splitter ends a sentence at "." only before whitespace or end of line, so "file.name is unique." is no longer split into "file" and a rule bound to the unrelated "name" column. Decimals are unaffected. - parse_confidence accepts a number only after a confidence word (confidence, certainty, probability, sure) or with "%"/"percent". Before, any bare number counted ("3 neighbours" became 0.03, "age > 50" became 0.5). Those conditions are now unparsed and raise under strict. - resolve_policy re-runs restatement and protection checks once it resolves constraints from a schema-free policy, and merges new issues without duplicates. fd.clean(policy=..., strict=True) now raises protection_conflict like the schema-bound flows; non-strict mode records the issue and demotes the repair. Closes #301 Closes #302 Closes #303 Closes #304 --- docs/context-policies.md | 15 +- src/freshdata/context/compiler.py | 13 +- src/freshdata/context/lexicon.py | 33 ++- src/freshdata/context/normalize.py | 65 +++++- src/freshdata/context/parser.py | 5 +- tests/context/test_compiler_parsing_fixes.py | 223 +++++++++++++++++++ 6 files changed, 338 insertions(+), 16 deletions(-) create mode 100644 tests/context/test_compiler_parsing_fixes.py diff --git a/docs/context-policies.md b/docs/context-policies.md index dce3d95c..bd27a86b 100644 --- a/docs/context-policies.md +++ b/docs/context-policies.md @@ -54,7 +54,20 @@ network call, and no randomness anywhere in this path. Confidence phrases are normalized: `>95%`, `confidence > 95%`, `confidence above 0.95`, and `only if confidence >= 95 percent` all compile to -`0.95`. +`0.95`. A number counts as a confidence only when a confidence word +(`confidence`, `certainty`, `probability`, `sure`) introduces it or it carries +`%`/`percent`; a condition such as `only if 3 neighbours agree` is reported as +unparsed rather than read as a gate. + +Allowed-values lists are split on `,` and `;` when present, with `and`/`or` +separating only the last item (`active, inactive or pending`); without commas, +`and`/`or` separate the values. `/` never splits a value, so `N/A` stays whole. +A value that contains `and`/`or` where a separator could be (for example +`Chile or Trinidad and Tobago`) is ambiguous: quote it, as in +`Allowed country values are Chile or 'Trinidad and Tobago'.` + +A period ends a sentence only before whitespace or the end of a line, so dotted +column names (`file.name is unique.`) and decimals stay in one sentence. ## Column references resolve against your real schema diff --git a/src/freshdata/context/compiler.py b/src/freshdata/context/compiler.py index fc0e8079..1ee16589 100644 --- a/src/freshdata/context/compiler.py +++ b/src/freshdata/context/compiler.py @@ -389,6 +389,7 @@ def resolve_policy( schema = [str(c) for c in columns] constraints: list[ColumnConstraint] = [] unresolved = list(policy.unresolved) + newly_resolved = False for c in policy.constraints: if c.rule == "dedup_key" and c.resolution_confidence == 0.0: resolved_cols: list[str] = [] @@ -414,6 +415,7 @@ def resolve_policy( resolved_cols.append(str(resolution.column)) if failed: continue + newly_resolved = True constraints.append( ColumnConstraint( id=c.id, @@ -450,6 +452,7 @@ def resolve_policy( constraint_params = dict(c.params) if resolution.method == "embedding": constraint_params["resolution_evidence"] = _resolution_evidence(resolution) + newly_resolved = True constraints.append( ColumnConstraint( id=c.id, @@ -463,12 +466,20 @@ def resolve_policy( provenance=c.provenance, ) ) + issues = list(policy.issues) + if newly_resolved: + # Columns only just became known, so the passes compile_context runs on + # resolved columns (restatement, protection-vs-repair) run now; the + # caller's strict check then sees any protection_conflict. + new_issues: list[PolicyIssue] = [] + constraints = _enforce_protection(_drop_superseded(constraints, new_issues), new_issues) + issues.extend(i for i in dict.fromkeys(new_issues) if i not in issues) return ContextPolicy( policy_version=policy.policy_version, dataset_domain=policy.dataset_domain, constraints=tuple(constraints), unresolved=tuple(unresolved), - issues=policy.issues, + issues=tuple(issues), source_text_sha256=policy.source_text_sha256, strict=policy.strict, ) diff --git a/src/freshdata/context/lexicon.py b/src/freshdata/context/lexicon.py index 058f751d..6a59aa90 100644 --- a/src/freshdata/context/lexicon.py +++ b/src/freshdata/context/lexicon.py @@ -173,30 +173,47 @@ def format_for(word: str) -> str | None: # Confidence phrases -> fraction in [0, 1]. # --------------------------------------------------------------------------- -_CONFIDENCE = re.compile( - r""" - (?:confidence|certainty|probability|sure(?:ness)?)? # optional subject - \s* +_COMPARATOR = r"(?:>=|>|≥|above|over|at\ least|exceeds|greater\ than|more\ than)?" +_VALUE = r"(?P\d+(?:\.\d+)?)" +_PERCENT = r"(?P%|percent\b|per\ cent\b)" + +#: A value introduced by a confidence subject: "confidence above 0.95". +_CONFIDENCE_SUBJECT = re.compile( + rf""" + \b(?:confidence|certainty|probability|sure(?:ness)?) + (?:\s+(?:level|score|threshold))? + \s*(?:[:=]\s*)? (?:is\s+|of\s+)? - (?:>=|>|≥|above|over|at\ least|exceeds|greater\ than|more\ than)? + {_COMPARATOR} \s* - (?P\d+(?:\.\d+)?) + {_VALUE} \s* - (?P%|percent|per\ cent)? + {_PERCENT}? """, re.IGNORECASE | re.VERBOSE, ) +#: A value that is explicitly a percentage: ">95%", "at least 90 percent". +_CONFIDENCE_PERCENT = re.compile( + rf"{_COMPARATOR}\s*{_VALUE}\s*{_PERCENT}", + re.IGNORECASE | re.VERBOSE, +) def parse_confidence(text: str) -> float | None: """Extract a confidence threshold from prose, normalized to [0, 1]. + A number counts only when a confidence word (confidence, certainty, + probability, sure) introduces it or it carries ``%``/``percent``; any + other number (``3 neighbours``, ``age > 50``) is not a confidence. + >>> parse_confidence(">95%"), parse_confidence("confidence above 0.95") (0.95, 0.95) >>> parse_confidence("only if confidence >= 95 percent") 0.95 + >>> parse_confidence("3 neighbours agree") is None + True """ - match = _CONFIDENCE.search(text) + match = _CONFIDENCE_SUBJECT.search(text) or _CONFIDENCE_PERCENT.search(text) if match is None: return None value = float(match.group("value")) diff --git a/src/freshdata/context/normalize.py b/src/freshdata/context/normalize.py index 249469a1..f792f039 100644 --- a/src/freshdata/context/normalize.py +++ b/src/freshdata/context/normalize.py @@ -69,11 +69,68 @@ def tokens(snake: str) -> tuple[str, ...]: return tuple(singular(t) for t in snake.split("_") if t) +#: A quoted value that starts and ends at a token boundary, so an apostrophe +#: inside a word (``Don't``) never opens a quote. +_QUOTED_VALUE = re.compile( + r"(?.*\S)\s+(?:and|or)\s+(?P\S.*)$") + + def split_value_list(raw: str) -> tuple[str, ...]: - """Split ``"active, inactive or pending"`` into cleaned value tokens.""" - cleaned = re.sub(r"[{}\[\]()]", "", raw) - parts = re.split(r",|;|\bor\b|\band\b|/", cleaned) - return tuple(p for p in (strip_quotes(x) for x in parts) if p) + """Split ``"active, inactive or pending"`` into cleaned value tokens. + + Splitting rules, applied in order: + + - A quoted value (``'N/A'``, ``"Trinidad and Tobago"``) is always kept + whole, and ``/`` never separates values. + - When the list contains ``,`` or ``;``, only those separate values; the + last item may still split once on a trailing ``and``/``or`` + (``a, b or c``), and a leading ``and``/``or`` (``a, b, and c``) is + dropped. + - Without ``,``/``;``, ``and``/``or`` separate values + (``email and phone``). + + A value that itself contains ``and``/``or`` in the position of a list + separator is ambiguous; quote it. + + >>> split_value_list("Trinidad and Tobago, Chile, N/A") + ('Trinidad and Tobago', 'Chile', 'N/A') + >>> split_value_list("active, inactive or pending") + ('active', 'inactive', 'pending') + >>> split_value_list("'Bosnia and Herzegovina' or Chile") + ('Bosnia and Herzegovina', 'Chile') + """ + quoted: list[str] = [] + + def _stash(match: re.Match[str]) -> str: + quoted.append(match.group(0)[1:-1].strip()) + return f"\x00{len(quoted) - 1}\x00" + + text = re.sub(r"[{}\[\]()]", "", _QUOTED_VALUE.sub(_stash, raw)) + if _LIST_SEPARATOR.search(text): + parts = [p.strip() for p in _LIST_SEPARATOR.split(text)] + last = _LEADING_CONJUNCTION.sub("", parts.pop()) + split_last = _TRAILING_CONJUNCTION.match(last) + if split_last is None: + parts.append(last) + else: + parts.extend([split_last.group("head"), split_last.group("tail")]) + else: + parts = _CONJUNCTION.split(text) + + def _restore(part: str) -> str: + return _PLACEHOLDER.sub(lambda m: quoted[int(m.group(1))], strip_quotes(part)) + + return tuple(v for v in (_restore(p) for p in parts) if v) def parse_scalar(raw: str) -> object: diff --git a/src/freshdata/context/parser.py b/src/freshdata/context/parser.py index 8349f2af..aa0edc0f 100644 --- a/src/freshdata/context/parser.py +++ b/src/freshdata/context/parser.py @@ -22,8 +22,9 @@ _NUM = r"-?\d+(?:\.\d+)?" _BULLET = re.compile(r"^\s*(?:[-*•]|\d+[.)])\s+") -#: Sentence enders; a period *between* digits (a decimal like 0.95) never splits. -_SPLIT = re.compile(r"[!?;]|(?<=\D)\.|\.(?=\D|$)") +#: Sentence enders. A period ends a sentence only before whitespace or the end +#: of the line, so decimals (``0.95``) and dotted names (``file.name``) survive. +_SPLIT = re.compile(r"[!?;]|\.(?=\s|$)") _WS = re.compile(r"\s+") diff --git a/tests/context/test_compiler_parsing_fixes.py b/tests/context/test_compiler_parsing_fixes.py new file mode 100644 index 00000000..21beba08 --- /dev/null +++ b/tests/context/test_compiler_parsing_fixes.py @@ -0,0 +1,223 @@ +"""Regression tests for context parsing and compile fixes (#301-#304).""" + +import warnings + +import pandas as pd +import pytest + +import freshdata as fd +from freshdata import PolicyError, compile_context +from freshdata.context import parse_context, resolve_policy, split_sentences +from freshdata.context.lexicon import parse_confidence +from freshdata.context.normalize import split_value_list + + +def _values(text, **kwargs): + policy = compile_context(text, **kwargs) + return {c.column: c.params["values"] for c in policy.constraints if c.rule == "allowed_values"} + + +# -- #301: allowed-values lists ------------------------------------------------------ + + +def test_issue_301_multiword_and_slash_values_stay_whole(): + df = pd.DataFrame({"country": ["Trinidad and Tobago", "Chile"], "code": ["N/A", "OK"]}) + text = ( + "Allowed country values are Trinidad and Tobago, Chile. " + "Allowed code values are N/A, OK." + ) + assert _values(text, df=df) == { + "country": ["Trinidad and Tobago", "Chile"], + "code": ["N/A", "OK"], + } + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + findings = fd.validate(df, context=text) + assert not [f for f in findings if f.rule_name == "context.allowed_values"] + + +@pytest.mark.parametrize( + ("raw", "expected"), + [ + ("active, inactive, pending", ("active", "inactive", "pending")), + ("active, inactive or pending", ("active", "inactive", "pending")), + ("active, inactive, or pending", ("active", "inactive", "pending")), + ("active, inactive, and pending", ("active", "inactive", "pending")), + ("male, female or other", ("male", "female", "other")), + ("email and phone", ("email", "phone")), + ("active or inactive", ("active", "inactive")), + ("a; b", ("a", "b")), + ("400001, 400002", ("400001", "400002")), + ("[a, b]", ("a", "b")), + ("CA, OR, WA", ("CA", "OR", "WA")), + ("N/A", ("N/A",)), + (",,", ()), + ], +) +def test_split_value_list_forms(raw, expected): + assert split_value_list(raw) == expected + + +def test_quoted_values_are_never_split(): + assert split_value_list("Chile or 'Trinidad and Tobago'") == ("Chile", "Trinidad and Tobago") + assert split_value_list("\"Bosnia and Herzegovina\", 'N/A (none)'") == ( + "Bosnia and Herzegovina", + "N/A (none)", + ) + assert split_value_list("['x', 'y']") == ("x", "y") + + +def test_apostrophes_inside_words_do_not_open_quotes(): + assert split_value_list("Don't know, won't say") == ("Don't know", "won't say") + + +def test_quoted_dotted_values_keep_their_periods(): + assert _values("Allowed country values are 'U.S.', 'U.K.'.", columns=["country"]) == { + "country": ["U.S.", "U.K."] + } + + +def test_dedup_key_split_is_unchanged(): + (candidate,) = parse_context("Deduplicate by email and phone.").candidates + assert candidate.column_refs == ("email", "phone") + (candidate,) = parse_context("Deduplicate by email, phone and city.").candidates + assert candidate.column_refs == ("email", "phone", "city") + + +# -- #302: sentence splitting ---------------------------------------------------------- + + +def test_issue_302_dotted_column_name_is_one_sentence(): + df = pd.DataFrame({"file.name": ["a", "b"], "name": ["x", "x"]}) + policy = compile_context( + "file.name is unique.", df=df, config=fd.CleanConfig(column_names=False) + ) + assert [(c.rule, c.column) for c in policy.constraints] == [("unique", "file.name")] + assert not policy.issues + + +def test_dotted_column_name_resolves_after_snake_casing(): + df = pd.DataFrame({"file.name": ["a", "b"], "name": ["x", "x"]}) + policy = compile_context("file.name is unique.", df=df) + assert [(c.rule, c.column) for c in policy.constraints] == [("unique", "file_name")] + + +def test_splitter_keeps_decimals_and_abbreviations(): + assert split_sentences( + "Allowed country values are U.S., U.K., Canada. Age must be between 0.5 and 1.5." + ) == ( + "Allowed country values are U.S., U.K., Canada", + "Age must be between 0.5 and 1.5", + ) + assert split_sentences("CustomerID is unique. Emails must be valid!\nProtect age.") == ( + "CustomerID is unique", + "Emails must be valid", + "Protect age", + ) + + +# -- #303: confidence gates ------------------------------------------------------------ + + +@pytest.mark.parametrize( + "text", + [ + "Impute missing income only if 3 neighbours agree", + "Fill missing age when age > 50", + "Missing age should be estimated only if 2 sources agree.", + ], +) +def test_issue_303_bare_numbers_are_not_confidence(text): + policy = compile_context(text, columns=["income", "age"]) + assert not policy.constraints + assert [i.kind for i in policy.issues] == ["unparsed_sentence"] + with pytest.raises(PolicyError, match="unparsed_sentence"): + compile_context(text, columns=["income", "age"], strict=True) + + +@pytest.mark.parametrize( + ("phrase", "expected"), + [ + (">95%", 0.95), + ("confidence > 95%", 0.95), + ("confidence above 0.95", 0.95), + ("only if confidence >= 95 percent", 0.95), + ("confidence >60%", 0.6), + ("certainty of at least 0.8", 0.8), + ("probability is over 90", 0.9), + ("at least 90% sure", 0.9), + ("confidence level of 0.7", 0.7), + ], +) +def test_explicit_confidence_phrases_still_parse(phrase, expected): + assert parse_confidence(phrase) == expected + + +@pytest.mark.parametrize("phrase", ["3 neighbours agree", "age > 50", "whenever it feels right"]) +def test_non_confidence_numbers_return_none(phrase): + assert parse_confidence(phrase) is None + + +def test_documented_impute_phrasings_still_compile(): + for text, expected in ( + ("Missing Age should be estimated only if confidence >95%.", 0.95), + ("Missing age must be imputed only when confidence above 0.95.", 0.95), + ("Estimate missing quantity only if confidence >= 90 percent.", 0.9), + ): + (c,) = compile_context(text, columns=["age", "quantity"]).constraints + assert c.params == {"min_confidence": expected}, text + + +# -- #304: protection conflicts after late resolution ---------------------------------- + +CONFLICT = "Never modify email. Emails must be valid." + + +def test_issue_304_schema_free_policy_raises_under_strict(): + df = pd.DataFrame({"email": ["a@b.com", "c@d.com"]}) + with pytest.raises(PolicyError, match="protection_conflict"): + compile_context(CONFLICT, columns=["email"], strict=True) + policy = compile_context(CONFLICT, strict=True) # schema-free: nothing to conflict yet + with pytest.raises(PolicyError, match="protection_conflict"): + fd.clean(df, policy=policy, strict=True, verbose=False) + + +def test_schema_free_resolution_matches_schema_bound_compile(): + bound = compile_context(CONFLICT, columns=["email"]) + late = resolve_policy(compile_context(CONFLICT), ["email"]) + assert [(c.rule, c.column, c.action) for c in late.constraints] == [ + (c.rule, c.column, c.action) for c in bound.constraints + ] + assert [(i.kind, i.severity, i.columns) for i in late.issues] == [ + (i.kind, i.severity, i.columns) for i in bound.issues + ] + repair = [c for c in late.constraints if c.rule == "valid_format"][0] + assert repair.action == "validate_only" + + +def test_non_strict_schema_free_policy_records_issue_and_protects(): + df = pd.DataFrame({"email": ["A@B.COM ", "c@d.com"]}) + policy = compile_context(CONFLICT) + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + out = fd.clean(df, policy=policy, verbose=False) + assert out["email"].tolist() == df["email"].tolist() + resolved = resolve_policy(policy, ["email"]) + assert [i.kind for i in resolved.issues] == ["protection_conflict"] + + +def test_late_resolution_detects_restatement_and_does_not_duplicate_issues(): + policy = compile_context("Age must be between 1 and 5. age must be between 2 and 6.") + assert not policy.issues # different surface refs: no restatement known yet + resolved = resolve_policy(policy, ["age"]) + (constraint,) = resolved.constraints + assert constraint.params == {"lo": 2, "hi": 6} + assert [i.kind for i in resolved.issues] == ["superseded"] + assert resolve_policy(resolved, ["age"]) is resolved + + +def test_resolve_policy_without_new_resolution_keeps_issues(): + policy = compile_context("heart_rate is unique. Gibberish here.") + resolved = resolve_policy(policy, ["age"]) + assert resolved.issues == policy.issues + assert not resolved.constraints