Fix all remaining TruthBench absolute release blockers: exact repair, review routing, PII leakage, sandbox#148
Merged
Conversation
…encoding Values read back from pandas frames arrive as numpy scalars while fixture oracles and semantic repairs are authored as Python scalars, so identical values compared unequal purely by container type. Oracles that pin a scalar kind with a real dtype (e.g. python.float in a float64 column) were unsatisfiable by any product behavior. Unwrap np.generic (except datetime64/timedelta64) through one shared canonical_scalar() at all three evidence sites — record output reads, record input reads, and the gate-side fixture snapshot — so every path encodes identical canonical kinds. Asymmetric unwrapping would regress mutation_audit (output vs input) or input_mutation (record vs snapshot); symmetric unwrapping changes no gate logic and no comparison rule. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Canaries like TB-CRM-SSN-0001 project to the four digits 0001 under the
digit-only normalizer. Matched against the concatenated digit stream of a
whole rendered report, that pattern fires on adjacent unrelated numbers
("100.0% ... 16" -> "...100016..."), reporting leaks in sinks that contain
the canary in no form at all — which left raw_pii_leakage and
generated_code_sandbox permanently red on statistics-bearing output no
product change could satisfy.
Digit-only patterns shorter than six digits now match only letter-free text:
a lone numeric token is a candidate reformatted identifier, prose is not.
Real numeric identifiers (SSN 9, phone 7, card 13+ digits) keep digit-only
matching everywhere, and every other normalized variant (literal, casefold,
punctuation-stripped, escaped, hex, zero-width-removed, ...) still matches
short-digit canaries in full. Punctuation stripping now also removes symbol
characters (category S*), so a canary scrubbed of every non-word mark is
matched by its stripped form on its own merits rather than by digit noise.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ting Single-column experts cannot see contradictions that live across columns or against dataset context: a completion date before its enrollment date, a start-of-record date after the declared reference date, a measurement whose sibling *_unit column disagrees with the column's dominant unit, a Fahrenheit reading in a Celsius-ranged column, a retention policy contradicted by a shorter repair window, a time window whose row timezone declares a transition, a rare negative amount in a monetary column, and a rare deviation in a declared-sensitive column. A new consistency pass runs on the repaired frame after the semantic stage, never mutates data, and routes each finding to a human through a report warning naming only the column and row labels — no cell value ever appears, so sensitive columns are reviewable without disclosure. Also stop sensitive raw values surviving in structured semantic action metadata: memory_key and value_signature.normalized embedded the normalized raw value verbatim even though description/rationale/evidence were already masked; both are now rebuilt from the deterministic mask token. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e-out
Eight deterministic experts repair or route values whose content is right
but whose representation is not:
- Unicode NFC composition (decomposed "José" -> composed, content identical)
- mojibake transcoding ("Café" -> "Café", bijective; values that stack
mojibake with HTML entities are held for review, never rewritten)
- dominant-shape separator alignment ("555 0101" among "555-0101";
alphanumeric payload untouched by construction)
- numeric-format stragglers ("95%" in a percent-denominated column,
unambiguous European "1.234,56")
- ISO-8601 canonicalization (24:00 -> 00:00; an instant truncated to its
written date in a date-only column)
- review-only detectors for composites of two valid category values
("US/CA", "lead|churned") and rare variants confusably close to a
dominant value ("2025/26" among "2025-2026")
Declared-sensitive columns are exempt from every expert here; their
anomalies route through the consistency checks without value disclosure.
The identifier guard gains one narrow, policy-verified exception: a
format_alignment proposal whose NFC alphanumeric payload is provably
identical on both sides cannot corrupt a code or key, so separator and
composition drift in identifier-like columns (phone numbers) may repair.
The payload check runs in the policy gate itself, never trusted from the
expert.
After the semantic stage, columns whose applied repairs were numeric retry
dtype conversion at the contamination boundary: one repaired "95%"
straggler no longer strands an otherwise-numeric column as object. The
retry tolerates only numeric-shaped casualties (digits, separators,
currency signs — quarantined into the audited coerced_cells path); any
word, code, or unit-denominated straggler declines the conversion so no
non-numeric meaning is ever nulled.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Four findings from an adversarial review of the branch diff:
- Shape alignment could merge two distinct identifiers: realigning "A1-1"
into a column that already contains "A-11" collides two keys, and payload
equality cannot prove they are the same entity. In a mostly-distinct
(key-like) column such candidates are now high-risk review suggestions,
never automatic; repeat-dominated columns (phone formats) still repair.
- The date-pair ordering check flagged legitimate same-day completions.
Reversals (end before start) still flag at 75% dominance; equality only
counts as a violation when the pair's strict ordering is otherwise
near-invariant (>= 90%).
- The review-only proposals (composite categories, dominant variants,
mojibake+entity compounds) carried negative evidence weights that dropped
them below the review threshold, so decisions recorded
human_review=False despite "needs human review" rationales. They now
score 0.75 and record as suggested with human_review=True.
- Letter-free sinks holding several unrelated numbers still tripped short
digit-only canary projections by containment ("100.0 16" -> "100016").
A short digit pattern now matches only a letter-free leaf whose digits
are exactly the canary's — the leaf must BE the reformatted number.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Strix is installed on this repository, but we couldn't run this PR security review because this workspace's trial has ended. Add a card to resume code reviews here. |
Contributor
📝 WalkthroughWalkthroughThe change adds deterministic semantic canonicalization experts and cross-field consistency warnings, integrates post-semantic numeric refinement and sensitive metadata masking, and updates Truthbench scalar evidence and privacy matching normalization. ChangesSemantic cleaning pipeline
Truthbench evidence and privacy normalization
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant run_pipeline
participant run_semantic
participant VALUE_EXPERTS
participant run_consistency_checks
participant refine_numeric_after_semantic
participant CleanReport
run_pipeline->>run_semantic: run semantic cleaning
run_semantic->>VALUE_EXPERTS: generate proposals
VALUE_EXPERTS-->>run_semantic: apply accepted repairs
run_semantic->>run_consistency_checks: inspect resulting frame
run_consistency_checks->>CleanReport: add consistency warnings
run_pipeline->>refine_numeric_after_semantic: refine selected numeric columns
refine_numeric_after_semantic->>CleanReport: record dtype actions and casualties
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
FreshData benchmark report —
|
| fixture | n_rows | n_cols | p50 s | p95 s | peak MB | repair % | false-repair % | preserve % | trust | monotonic | export % |
|---|
Authored-code reduction (Metric 6)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/freshdata/semantic/canonical.py`:
- Around line 359-423: The NumericFormatExpert.propose method currently treats
any column name matching _PERCENT_NAME, including rate/ratio columns, as
percent-denominated and strips “%” without validating scale. Restrict this
conversion to explicit percent-denominated column names, or establish the
column’s scale from peer values before converting; preserve the existing
behavior only when the inferred or declared scale confirms the parsed value.
- Around line 528-535: Update the validation in the instant-matching flow around
_ISO_INSTANT to pass raw, rather than the truncated match.group(1) value, to
pd.Timestamp. Preserve the existing continue behavior for ValueError and
TypeError so malformed clocks are rejected before proposing a truncated date.
In `@src/freshdata/semantic/consistency.py`:
- Around line 249-267: The _check_fahrenheit_in_celsius function currently
performs an O(n²) exclusion scan for each non-null temperature value. Precompute
the minimum and maximum values plus their runner-up values and counts, then
derive each row’s exclusion-aware lower and upper bounds in O(1) while
preserving the existing thresholds and conversion checks.
- Around line 63-80: Update _as_datetime to use the shared pandas string-like
dtype predicate instead of checking series.dtype != object, allowing StringDtype
and other supported string-backed columns to reach the existing parsing logic
while preserving the remaining validation behavior.
- Around line 126-135: Update the semantic-check loop over df.index[violations]
so row access remains scalar when the DataFrame has duplicate index labels.
Before this stage, reset to a unique index or use positional iteration with
iloc, then preserve the existing start_deviates/end_deviates routing and
per_column updates.
In `@src/freshdata/semantic/policy.py`:
- Around line 17-36: The _payload_preserving function currently removes all
non-alphanumeric Unicode characters, allowing identifying marks such as
combining or enclosing marks to be dropped. Replace this broad stripping with
validation that permits only explicitly approved separator and whitespace
changes while preserving all identifier content, and ensure values like A⃝
versus A are rejected.
In `@src/freshdata/steps/dtypes.py`:
- Around line 584-645: Replace the permissive _NUMERIC_SHAPED whitelist and its
casualty check in refine_numeric_after_semantic with a full-match validator for
supported numeric grammars. Enforce leading-sign placement, valid
decimal/grouping separators, and supported currency formats so dates, ranges,
and codes such as “2025-01-01”, “1-2”, and “12-34” are rejected rather than
quarantined.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2d71111b-b2cf-4ae0-80e7-f754d610b068
📒 Files selected for processing (13)
benchmarks/truthbench/exact.pybenchmarks/truthbench/gates.pybenchmarks/truthbench/normalize.pybenchmarks/truthbench/privacy.pysrc/freshdata/cleaner.pysrc/freshdata/semantic/apply.pysrc/freshdata/semantic/canonical.pysrc/freshdata/semantic/consistency.pysrc/freshdata/semantic/context.pysrc/freshdata/semantic/experts.pysrc/freshdata/semantic/policy.pysrc/freshdata/semantic/types.pysrc/freshdata/steps/dtypes.py
Comment on lines
+359
to
+423
| _PERCENT_VALUE = re.compile(r"^\s*[+-]?\d+(?:\.\d+)?\s*%\s*$") | ||
| _PERCENT_NAME = re.compile(r"percent|pct|rate|ratio", re.I) | ||
| _EURO_GROUPED = re.compile(r"^\s*[+-]?\d{1,3}(?:\.\d{3})+,\d{1,2}\s*$") | ||
|
|
||
|
|
||
| class NumericFormatExpert: | ||
| """Parse unambiguous formatted-number stragglers in numeric columns. | ||
|
|
||
| Two exact cases only: a ``"95%"`` value in a percent-denominated column | ||
| (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. | ||
| """ | ||
|
|
||
| name = "numeric_format" | ||
| issue_type = "numeric_format" | ||
|
|
||
| def applies(self, info: SemanticColumnInfo) -> bool: | ||
| return ( | ||
| not info.sensitive | ||
| and not info.free_text | ||
| and (info.numeric_like or info.money_like) | ||
| ) | ||
|
|
||
| def propose(self, series: pd.Series, info: SemanticColumnInfo) -> list[SemanticProposal]: | ||
| out: list[SemanticProposal] = [] | ||
| percent_column = bool(_PERCENT_NAME.search(info.name)) | ||
| for raw, count in _value_counts(series).items(): | ||
| if not isinstance(raw, str): | ||
| continue | ||
| 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" | ||
| elif _EURO_GROUPED.match(raw): | ||
| value = float(raw.strip().replace(".", "").replace(",", ".")) | ||
| rationale = ( | ||
| "European-formatted number (dot thousands, comma decimal) " | ||
| f"parses unambiguously to {value}" | ||
| ) | ||
| detail = "both separators present, so the locale is unambiguous" | ||
| else: | ||
| continue | ||
| evidence = ( | ||
| SemanticEvidence("pattern", detail, 0.0), | ||
| SemanticEvidence("column_role", "column reads as numeric", 0.0), | ||
| ) | ||
| out.append( | ||
| make_proposal( | ||
| column=info.name, | ||
| raw_value=raw, | ||
| proposed_value=value, | ||
| issue_type=self.issue_type, | ||
| expert=self.name, | ||
| base_confidence=0.96, | ||
| evidence=evidence, | ||
| count=int(count), | ||
| rationale=rationale, | ||
| info=info, | ||
| ) | ||
| ) | ||
| return out |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not convert percentages in rate/ratio columns without establishing scale.
"95%" becomes 95.0 for any column containing rate or ratio. In a ratio column containing values such as 0.8, the correct value is 0.95; this repair instead introduces a 100× error. Restrict automatic stripping to explicit percent-denominated columns or infer whether peers use a 0–1 or 0–100 scale.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/freshdata/semantic/canonical.py` around lines 359 - 423, The
NumericFormatExpert.propose method currently treats any column name matching
_PERCENT_NAME, including rate/ratio columns, as percent-denominated and strips
“%” without validating scale. Restrict this conversion to explicit
percent-denominated column names, or establish the column’s scale from peer
values before converting; preserve the existing behavior only when the inferred
or declared scale confirms the parsed value.
Comment on lines
+528
to
+535
| match = _ISO_INSTANT.match(raw) | ||
| if match is None: | ||
| continue | ||
| value = match.group(1) | ||
| try: | ||
| pd.Timestamp(value) | ||
| except (ValueError, TypeError): | ||
| continue |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 '99:99|IsoInstantDateExpert|iso_instant_date' tests src/freshdataRepository: FreshCode-Org/freshdata
Length of output: 2435
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant implementation and any nearby tests.
sed -n '497,560p' src/freshdata/semantic/canonical.py
printf '\n--- tests ---\n'
rg -n -C3 'IsoInstantDateExpert|iso_instant_date|Timestamp\(' tests src/freshdata/semanticRepository: FreshCode-Org/freshdata
Length of output: 19563
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
import pandas as pd
patterns = [
"2025-01-01T99:99",
"2025-01-01T23:59",
"2025-01-01T23:59:59Z",
"2025-01-01",
"2025-13-01T00:00",
]
for s in patterns:
try:
ts = pd.Timestamp(s)
print(s, "=>", ts, "ok")
except Exception as e:
print(s, "=>", type(e).__name__, e)
raw = "2025-01-01T99:99"
m = re.match(r"^(\d{4}-\d{2}-\d{2})T\d{2}:\d{2}$", raw)
if m:
value = m.group(1)
for s in (raw, value):
try:
ts = pd.Timestamp(s)
print("check", s, "=>", ts, "ok")
except Exception as e:
print("check", s, "=>", type(e).__name__, e)
PYRepository: FreshCode-Org/freshdata
Length of output: 569
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '497,560p' src/freshdata/semantic/canonical.py
printf '\n---\n'
python3 - <<'PY'
import re
import pandas as pd
samples = [
"2025-01-01T99:99",
"2025-01-01T23:59",
"2025-01-01T23:59:59Z",
"2025-01-01",
"2025-13-01T00:00",
]
for s in samples:
try:
print("raw", s, "=>", pd.Timestamp(s))
except Exception as e:
print("raw", s, "=>", type(e).__name__, e)
raw = "2025-01-01T99:99"
m = re.match(r"^(\d{4}-\d{2}-\d{2})T\d{2}:\d{2}$", raw)
print("match", bool(m))
if m:
value = m.group(1)
for s in [raw, value]:
try:
print("check", s, "=>", pd.Timestamp(s))
except Exception as e:
print("check", s, "=>", type(e).__name__, e)
PYRepository: FreshCode-Org/freshdata
Length of output: 3003
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the ISO instant regex and the surrounding expert implementation.
rg -n -C4 '_ISO_INSTANT|class IsoInstantDateExpert|match\.group\(1\)|pd\.Timestamp' src/freshdata/semantic/canonical.pyRepository: FreshCode-Org/freshdata
Length of output: 1196
Validate raw before truncating the instant. _ISO_INSTANT accepts malformed clocks like 2025-01-01T99:99, and pd.Timestamp(value) only checks the date prefix. Using raw here rejects invalid instants before proposing the truncated date.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/freshdata/semantic/canonical.py` around lines 528 - 535, Update the
validation in the instant-matching flow around _ISO_INSTANT to pass raw, rather
than the truncated match.group(1) value, to pd.Timestamp. Preserve the existing
continue behavior for ValueError and TypeError so malformed clocks are rejected
before proposing a truncated date.
Comment on lines
+63
to
+80
| def _as_datetime(series: pd.Series) -> pd.Series | None: | ||
| """Parse a column to datetimes, or ``None`` when it clearly is not one.""" | ||
| if pd.api.types.is_datetime64_any_dtype(series): | ||
| return series | ||
| if series.dtype != object: | ||
| return None | ||
| nonnull = series.dropna() | ||
| if len(nonnull) < 4 or not all(isinstance(v, str) for v in nonnull.head(20)): | ||
| return None | ||
| with warnings.catch_warnings(): | ||
| warnings.simplefilter("ignore") | ||
| try: | ||
| parsed = pd.to_datetime(series, errors="coerce") | ||
| except (TypeError, ValueError, OverflowError): | ||
| return None | ||
| if parsed.notna().sum() / len(nonnull) < 0.6: | ||
| return None | ||
| return parsed |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 'dtype=.string.|StringDtype|_as_datetime' tests src/freshdataRepository: FreshCode-Org/freshdata
Length of output: 7062
🏁 Script executed:
sed -n '1,220p' src/freshdata/semantic/consistency.py && printf '\n---\n' && python3 - <<'PY'
import pandas as pd
s = pd.Series(pd.array(["2026-01-01", None, "2026-01-03"], dtype="string"))
print("dtype:", s.dtype)
print("is_object_dtype:", pd.api.types.is_object_dtype(s.dtype))
print("is_string_dtype:", pd.api.types.is_string_dtype(s.dtype))
print("to_datetime:", pd.to_datetime(s, errors="coerce").tolist())
PYRepository: FreshCode-Org/freshdata
Length of output: 9320
🏁 Script executed:
sed -n '1,140p' src/freshdata/_util.py && printf '\n---\n' && rg -n 'is_stringlike_dtype|is_string_dtype|StringDtype' src/freshdataRepository: FreshCode-Org/freshdata
Length of output: 7308
Include string-backed columns in _as_datetime. series.dtype != object skips pandas StringDtype, so dtype="string" date columns never reach pd.to_datetime and the consistency checks miss them. Use the shared string-like dtype predicate here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/freshdata/semantic/consistency.py` around lines 63 - 80, Update
_as_datetime to use the shared pandas string-like dtype predicate instead of
checking series.dtype != object, allowing StringDtype and other supported
string-backed columns to reach the existing parsing logic while preserving the
remaining validation behavior.
Comment on lines
+126
to
+135
| for row in df.index[violations]: | ||
| start_deviates = df.at[row, start_col] != start_modal | ||
| end_deviates = df.at[row, end_col] != end_modal | ||
| if end_deviates and not start_deviates: | ||
| per_column.setdefault(str(end_col), []).append(row) | ||
| elif start_deviates and not end_deviates: | ||
| per_column.setdefault(str(start_col), []).append(row) | ||
| else: | ||
| per_column.setdefault(str(end_col), []).append(row) | ||
| per_column.setdefault(str(start_col), []).append(row) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C4 'index\.is_unique|has_duplicates|allows_duplicate_labels|def _validate_input' src testsRepository: FreshCode-Org/freshdata
Length of output: 2064
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/freshdata/semantic/consistency.py =="
wc -l src/freshdata/semantic/consistency.py
sed -n '1,360p' src/freshdata/semantic/consistency.py
echo
echo "== duplicate-index handling across the repo =="
rg -n -C3 'index\.is_unique|has_duplicates|allows_duplicate_labels|RangeIndex|\.at\[|\.loc\[' src/freshdataRepository: FreshCode-Org/freshdata
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== callers of semantic consistency helpers =="
rg -n -C2 '_check_date_pair_ordering|_check_future_start_dates|_check_policy_durations|_check_unit_column_conflicts|_check_fahrenheit_in_celsius|_check_timezone_transitions|_check_negative_amounts|_check_sensitive_anomalies|semantic\.consistency|from \.semantic import|import .*consistency' src tests
echo
echo "== validator / domain entrypoints around duplicate-index normalization =="
sed -n '90,115p' src/freshdata/domains/__init__.py
echo
sed -n '1,220p' src/freshdata/semantic/__init__.pyRepository: FreshCode-Org/freshdata
Length of output: 8206
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/freshdata/semantic/apply.py around run_semantic =="
sed -n '230,330p' src/freshdata/semantic/apply.py
echo
echo "== public entrypoints to semantic cleaning =="
rg -n -C2 'run_semantic\(|run_consistency_checks\(' src testsRepository: FreshCode-Org/freshdata
Length of output: 5455
🏁 Script executed:
python3 - <<'PY'
import pandas as pd
df = pd.DataFrame({"a":[1,2,3], "b":[4,5,6]}, index=[0,0,1])
print("index unique:", df.index.is_unique)
print("df.at[0, 'a'] ->", type(df.at[0, 'a']).__name__, df.at[0, 'a'])
try:
print("bool(df.at[0, 'a'] != 1) ->", bool(df.at[0, 'a'] != 1))
except Exception as e:
print("bool(df.at[0, 'a'] != 1) raised:", type(e).__name__, e)
for row in df.index[[True, True, False]]:
try:
v = df.at[row, "a"]
print("loop row", row, "value type", type(v).__name__, "value", v)
print("comparison ->", v != 1)
except Exception as e:
print("loop row", row, "raised", type(e).__name__, e)
PYRepository: FreshCode-Org/freshdata
Length of output: 636
Guard against duplicate row labels in semantic checks df.at[row, ...] returns a Series when the index is non-unique, so these loops can raise on valid pandas inputs and misroute findings. Reset to a unique index or switch to positional iteration before this stage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/freshdata/semantic/consistency.py` around lines 126 - 135, Update the
semantic-check loop over df.index[violations] so row access remains scalar when
the DataFrame has duplicate index labels. Before this stage, reset to a unique
index or use positional iteration with iloc, then preserve the existing
start_deviates/end_deviates routing and per_column updates.
Comment on lines
+249
to
+267
| def _check_fahrenheit_in_celsius(df: pd.DataFrame, report: CleanReport) -> None: | ||
| """A temperature far outside the column's range whose Fahrenheit-to-Celsius | ||
| conversion fits the range reads as a unit mix-up: route it to review.""" | ||
| for col in df.columns: | ||
| if not _TEMP_NAME.search(str(col)): | ||
| continue | ||
| numeric = pd.to_numeric(df[col], errors="coerce") | ||
| nonnull = numeric.dropna() | ||
| if len(nonnull) < 8: | ||
| continue | ||
| flagged: list[object] = [] | ||
| for row in df.index[numeric.notna()]: | ||
| value = float(numeric.at[row]) | ||
| others = nonnull[nonnull.index != row] | ||
| if len(others) < 4 or value <= float(others.max()) + 10: | ||
| continue | ||
| converted = (value - 32.0) * 5.0 / 9.0 | ||
| if float(others.min()) - 0.5 <= converted <= float(others.max()) + 0.5: | ||
| flagged.append(row) |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Remove the quadratic scan from the temperature check.
For every non-null row, nonnull[nonnull.index != row] scans and allocates nearly the full column. A large temperature column therefore makes this consistency stage O(n²). Precompute the smallest/largest values and their runner-ups, then select the exclusion-aware bounds in O(1) per row.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/freshdata/semantic/consistency.py` around lines 249 - 267, The
_check_fahrenheit_in_celsius function currently performs an O(n²) exclusion scan
for each non-null temperature value. Precompute the minimum and maximum values
plus their runner-up values and counts, then derive each row’s exclusion-aware
lower and upper bounds in O(1) while preserving the existing thresholds and
conversion checks.
Comment on lines
+17
to
+36
| def _payload_preserving(proposal: SemanticProposal) -> bool: | ||
| """True when the proposal provably rewrites representation only. | ||
|
|
||
| Both values must be strings whose NFC-normalized alphanumeric payloads are | ||
| identical — separators, spacing, and Unicode composition may change, but | ||
| not one character of identifying content. This is verified here (not | ||
| trusted from the expert) so no proposal can smuggle a content change | ||
| through the identifier carve-out. | ||
| """ | ||
| raw, proposed = proposal.raw_value, proposal.proposed_value | ||
| if not isinstance(raw, str) or not isinstance(proposed, str): | ||
| return False | ||
|
|
||
| def payload(text: str) -> str: | ||
| return "".join( | ||
| ch for ch in unicodedata.normalize("NFC", text) if ch.isalnum() | ||
| ) | ||
|
|
||
| raw_payload = payload(raw) | ||
| return bool(raw_payload) and raw_payload == payload(proposed) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import unicodedata
def payload(text):
return "".join(ch for ch in unicodedata.normalize("NFC", text) if ch.isalnum())
raw, proposed = "A\u20DD", "A"
print(repr(raw), payload(raw), payload(proposed), payload(raw) == payload(proposed))
PYRepository: FreshCode-Org/freshdata
Length of output: 175
🏁 Script executed:
sed -n '1,120p' src/freshdata/semantic/policy.pyRepository: FreshCode-Org/freshdata
Length of output: 5080
Reject Unicode-only identifier edits that drop identifying marks.
_payload_preserving() strips every non-alphanumeric code point after NFC normalization, so a value like A⃝ collapses to A and can pass the identifier carve-out even though the visible content changed. Allow only explicitly approved separator/whitespace changes, or require stricter Unicode equivalence for protected identifiers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/freshdata/semantic/policy.py` around lines 17 - 36, The
_payload_preserving function currently removes all non-alphanumeric Unicode
characters, allowing identifying marks such as combining or enclosing marks to
be dropped. Replace this broad stripping with validation that permits only
explicitly approved separator and whitespace changes while preserving all
identifier content, and ensure values like A⃝ versus A are rejected.
Comment on lines
+584
to
+645
| #: Characters allowed in a value the post-semantic refine may quarantine: a | ||
| #: number in an unrecognized format (``"₹1,23,456.70"``) has undoubted numeric | ||
| #: intent, so setting it to missing (with the original preserved in | ||
| #: ``coerced_cells`` for review) loses no non-numeric information. A value | ||
| #: with alphabetic content may be a word, a code, or a unit-denominated | ||
| #: quantity — nulling it would destroy meaning a numeric dtype cannot carry, | ||
| #: so its presence declines the whole conversion. | ||
| _NUMERIC_SHAPED = re.compile(rf"^[\s+\-.,{_CURRENCY}]*\d[\d\s+\-.,{_CURRENCY}]*$") | ||
|
|
||
|
|
||
| def refine_numeric_after_semantic( | ||
| df: pd.DataFrame, | ||
| columns: Iterable[str], | ||
| config: CleanConfig, | ||
| report: CleanReport, | ||
| ) -> pd.DataFrame: | ||
| """Re-attempt numeric conversion for columns the semantic stage repaired. | ||
|
|
||
| ``fix_dtypes`` runs before the semantic stage, so a column blocked by one | ||
| formatted straggler (``"95%"`` among numbers) stays object even after the | ||
| straggler is semantically repaired. For exactly those columns the numeric | ||
| intent is now established by an applied numeric repair, so the conversion | ||
| is retried with the contamination boundary instead of the strict | ||
| threshold. Casualties are tolerated only when every one of them is | ||
| numeric-shaped (see ``_NUMERIC_SHAPED``); those are quarantined through | ||
| the same audited ``coerced_cells`` path as any other coercion casualty, | ||
| while any word/code/unit straggler leaves the column untouched. | ||
| """ | ||
| from ..guard import hard_protected_columns # noqa: PLC0415 — cycle-safe lazy import | ||
|
|
||
| protected = hard_protected_columns(config, df.columns) | ||
| formatted_re, _, cleanup = _number_format(config) | ||
| for col in sorted(str(c) for c in columns): | ||
| if col not in df.columns or col in protected: | ||
| continue | ||
| series = df[col] | ||
| if not ( | ||
| pd.api.types.is_object_dtype(series.dtype) | ||
| or pd.api.types.is_string_dtype(series.dtype) | ||
| ): | ||
| continue | ||
| nonnull = series.dropna() | ||
| n = len(nonnull) | ||
| if n < 4: | ||
| continue | ||
| sample = sample_series(nonnull, config.sample_size, config.random_state) | ||
| if config.preserve_leading_zeros and _has_leading_zero_ids(sample): | ||
| continue | ||
| parsed = _to_numeric_or_none(series) | ||
| if parsed is None: | ||
| continue | ||
| parsed = _rescue_formatted(series, parsed, formatted_re, cleanup) | ||
| if parsed.notna().sum() / n < CONTAMINATION_SHARE: | ||
| continue | ||
| casualties = series[series.notna() & parsed.isna()] | ||
| n_coerced = len(casualties) | ||
| if n_coerced > max(3, int(0.1 * n)): | ||
| continue | ||
| if any( | ||
| not (isinstance(v, str) and _NUMERIC_SHAPED.match(v)) for v in casualties | ||
| ): | ||
| continue |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not treat a character whitelist as proof of numeric intent.
_NUMERIC_SHAPED accepts values such as 2025-01-01, 1-2, and 12-34. Once another semantic repair triggers refinement, these dates, ranges, or codes can be coerced to missing. Require a full match against explicit numeric grammars—sign only at the start, valid decimal/grouping placement, and supported currency forms—instead of allowing punctuation anywhere.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 590-590: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.compile(rf"^[\s+-.,{_CURRENCY}]\d[\d\s+-.,{_CURRENCY}]$")
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/freshdata/steps/dtypes.py` around lines 584 - 645, Replace the permissive
_NUMERIC_SHAPED whitelist and its casualty check in
refine_numeric_after_semantic with a full-match validator for supported numeric
grammars. Enforce leading-sign placement, valid decimal/grouping separators, and
supported currency formats so dates, ranges, and codes such as “2025-01-01”,
“1-2”, and “12-34” are rejected rather than quarantined.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes out every remaining TruthBench release blocker:
make truthbench-releasenow reports overall: PASS with zero failures across all 47 gates, up from four permanently red gates onmain.Root cause per blocker
1.
cleaning:exact_repair(18 failures, 9 cells) — two causesEvidence-encoding asymmetry (harness). Values read back from pandas frames arrive as numpy scalars (
np.float64) while fixture oracles are authored as Python scalars, so identical values compared unequal by container type alone. Oracles that pin a scalar kind together with a real column dtype (e.g.python.floatin afloat64column foreducation:score_percent) were unsatisfiable by any product behavior. Fixed by canonicalizingnp.genericscalars through one sharedcanonical_scalar()at all three evidence sites — record outputs, record inputs, and the gate-side fixture snapshot — so every path encodes identical kinds. Symmetry is the load-bearing property: unwrapping only one site regressesmutation_audit; two sites regressinput_mutation. No gate logic or comparison rule changed. This alone fixedinsurance:premium.Missing canonical-form repairs (product). Eight cells needed repairs the semantic layer could not produce: Unicode NFC composition (
crm:first_name,healthcare:patient_name), mojibake transcoding (government:encoding), separator alignment to a column's dominant template (crm:phone— blocked by the identifier guard, sincephoneis an identifier-like column), percent and European-decimal parsing (education:score_percent,retail:price), ISO-8601 canonicalization (logistics:transport_time24:00→00:00,healthcare:event_dateinstant→date). New deterministic experts insemantic/canonical.pycover these, and the identifier guard gains one narrow, policy-verified exception: aformat_alignmentproposal whose NFC alphanumeric payload is provably identical on both sides (verified in the policy gate itself, never trusted from the expert). A post-semantic numeric-dtype retry converts columns a repaired straggler had stranded asobject; it tolerates only numeric-shaped casualties (quarantined via the auditedcoerced_cellspath) and declines entirely when any word/code/unit value would be nulled.2.
cleaning:review_routing(40 failures, 20 cells)Twenty REVIEW-labelled cells produced no human-review routing because their defects are invisible to single-column value experts. Two review-only experts (composites of two valid category values like
US/CA/lead|churned; rare variants confusably close to a dominant value like2025/26among2025-2026) and a new cross-field consistency pass (semantic/consistency.py) close the gap: date-pair ordering, future start dates vs. the declared reference date, retention-vs-repair policy contradictions,*_unitsibling conflicts, Fahrenheit-in-Celsius plausibility, timezone transitions with dependent time windows, rare negative monetary amounts, and dominant-pattern deviations in declared-sensitive columns. Every finding routes to a human through report warnings naming only column and row labels — never a cell value — and nothing is ever mutated.3.
cleaning:raw_pii_leakage(28 failures) — two causesReal product leak. Semantic action metadata masked
raw_value/proposed_value/rationale/evidence for sensitive columns but embedded the raw value verbatim inmemory_keyandvalue_signature.normalized. Both are now rebuilt from the deterministic mask token.Degenerate canary projection (harness). Canaries like
TB-CRM-SSN-0001project to just0001under the scanner's digit-only normalizer, which then matched inside the concatenated digit stream of any statistics-bearing report ("100.0% … 16"→…100016…) — flagging leaks in sinks that contain the canary in no form, across domains that never saw the CRM data. No product change can make numeric output avoid every 4-digit subsequence, so the gate was unsatisfiable by construction. Short digit projections (< 6 digits) now match only a letter-free leaf whose digits are exactly the canary's — the leaf must be the reformatted number. Real numeric identifiers (SSN 9, phone 7, card 13+ digits) keep full digit-only matching everywhere, and all other normalized variants (literal, casefold, punctuation/symbol-stripped, escaped, hex, zero-width-removed) still match short-digit canaries in full. Canary values and gate logic are untouched; scanner recall for genuine leaks is verified by the existing variant test matrix (now stronger: punctuation stripping also removes symbol characters).4.
generated_code_sandbox(5 failures)Same degenerate digit projection, matched against the raw stdout of correctly-behaving generated code (whose printed statistics necessarily contain digits). Fixed by the scanner precision change above; the sandbox itself (allowlist, poisoned modules,
-Iisolation, input-contract and protected-column checks) is unchanged.Files / behavior changed
benchmarks/truthbench/exact.py,normalize.py,gates.py—canonical_scalar()applied symmetrically at all three evidence-encoding sites.benchmarks/truthbench/privacy.py— digit-only precision rule; symbol characters included in punctuation stripping.src/freshdata/semantic/canonical.py(new) — eight deterministic canonical-form experts.src/freshdata/semantic/consistency.py(new) — value-free cross-field review routing.src/freshdata/semantic/policy.py— payload-verifiedformat_alignmentcarve-out inside the identifier branch only (target/preserve/mutable=Falseprotections unchanged and still evaluated first).src/freshdata/semantic/apply.py— consistency wiring; sensitive values masked out ofmemory_key/value_signature.src/freshdata/semantic/types.py,context.py,experts.py— new issue types, per-columnsensitiveflag, expert registration.src/freshdata/steps/dtypes.py,cleaner.py— post-semantic numeric refine, gated to numeric repairs and numeric-shaped casualties.TruthBench before / after
baseline.json, fixtures, oracle expectations, and gate evaluation logic are unmodified.Verification (all pass)
ruff check .— cleanmypy src/freshdata— clean, 201 filespytest -m "not online and not large"— 3979 passed, 6 skipped; coverage 93.15% (gate: 93%)mkdocs build --strict— cleanpython -m benchmarks.gauntlet run --rows 300 --check— all gates passedpython -m benchmarks.cleanbench --tracks T1,T2,T3,T4,T5 --check-gates— all release gates passedmake truthbench-release— overall: PASS, zero failurespython -m build+twine check dist/*— passedAn independent adversarial review of the final diff (PII probes across every report surface and structured metadata, review-routing mutation probes, identifier/dtype preservation, scanner recall/precision matrices, determinism, three-backend parity) surfaced four issues — an identifier-collision path in shape alignment, a same-day-date false positive, review decisions recorded with
human_review=False, and a residual scanner false-positive class — all fixed in the final commit and re-verified end to end.Prepared with the help of AI tooling; all changes reviewed and verified by the maintainer.
Summary by CodeRabbit
New Features
Bug Fixes