diff --git a/benchmarks/truthbench/fixtures/logistics.py b/benchmarks/truthbench/fixtures/logistics.py index 452b7ffa..92533999 100644 --- a/benchmarks/truthbench/fixtures/logistics.py +++ b/benchmarks/truthbench/fixtures/logistics.py @@ -138,10 +138,21 @@ def build(seed: int = 1729) -> TruthFixture: "log-06", "transport_time", "24:00", - Disposition.REPAIR, - expected="00:00", + # Corrected oracle: the column holds clock times without dates, so + # 00:00 would move end-of-day to start-of-day; routed to a human. + Disposition.REVIEW, family="twentyfour-hour-transport", ) + # The domain's exact repair: a status label whose separator drifted from + # the column's single dominant spelling has one correct form. + builder.inject( + "log-07", + "tracking_status", + "on time", + Disposition.REPAIR, + expected="on-time", + family="spaced-tracking-status", + ) builder.inject( "log-09", "address", diff --git a/src/freshdata/semantic/canonical.py b/src/freshdata/semantic/canonical.py index 37314ffa..43432db9 100644 --- a/src/freshdata/semantic/canonical.py +++ b/src/freshdata/semantic/canonical.py @@ -543,7 +543,14 @@ def propose(self, series: pd.Series, info: SemanticColumnInfo) -> list[SemanticP class TimeCanonicalExpert: - """Canonicalize ISO-8601 end-of-day ``24:00`` to ``00:00`` in time columns.""" + """Propose ``00:00`` for an end-of-day ``24:00`` in a time column, for review. + + ``24:00`` on day D is the same instant as ``00:00`` on day D+1 only when a + date travels with the value. These columns hold clock times alone, so the + rewrite would turn end-of-day into start-of-day (a ``shift_end`` of + ``24:00`` would sort before its ``shift_start``). The proposal is scored + below the auto-apply threshold and is always held for a human. + """ name = "time_canonical" issue_type = "format_alignment" @@ -585,12 +592,12 @@ 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=0.80, evidence=evidence, count=int(count), rationale=( - "ISO 8601 end-of-day 24:00 canonicalizes to midnight " - "00:00; the instant is unchanged" + "24:00 marks end of day; in a time-only column 00:00 " + "would read as start of day, so this is held for review" ), info=info, ) diff --git a/tests/test_semantic_repair_safety.py b/tests/test_semantic_repair_safety.py index c6c91c47..43f74944 100644 --- a/tests/test_semantic_repair_safety.py +++ b/tests/test_semantic_repair_safety.py @@ -203,10 +203,29 @@ def test_replay_expert_falls_back_to_issue_type_only_when_unambiguous() -> None: "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"]}, } +def test_suggested_time_canonical_repair_is_not_learned() -> None: + # 24:00 -> 00:00 in a time-only column is only suggested (#305), and + # suggestions never enter cleaning memory, so nothing replays. + data = {"start_time": ["09:00", "10:30", "11:15", "12:00", "24:00"]} + df = pd.DataFrame(data) + out, report = _clean(df) + assert out["start_time"].iloc[4] == "24:00" + actions = [a for a in _semantic(report) if a.metadata.get("expert") == "time_canonical"] + assert [a.status for a in actions] == ["suggested"] + + memory = fd.learn_cleaning_memory(df, decisions=report, dataset_id="d") + assert memory.value_patterns.get("semantic_repairs", []) == [] + ctx = build_semantic_context(df, CleanConfig(semantic_mode="auto")) + assert semantic_memory_proposals(df, ctx, memory).proposals == [] + + replayed, replay_report = _clean(pd.DataFrame(data), memory=memory) + assert replayed["start_time"].iloc[4] == "24:00" + assert not any(a.memory_influenced for a in _semantic(replay_report)) + + @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] diff --git a/tests/test_semantic_time_24h.py b/tests/test_semantic_time_24h.py new file mode 100644 index 00000000..c44a2336 --- /dev/null +++ b/tests/test_semantic_time_24h.py @@ -0,0 +1,79 @@ +"""#305: ``24:00`` in a date-less time column is suggested, never auto-applied. + +Without a date, ``24:00`` (end of day) and ``00:00`` (start of day) are +different clock times, so the rewrite is held for review in every mode. +""" + +from __future__ import annotations + +import pandas as pd +import pytest + +import freshdata as fd +from freshdata.config import CleanConfig +from freshdata.semantic.canonical import TimeCanonicalExpert +from freshdata.semantic.context import build_semantic_context + +_RATIONALE = ( + "24:00 marks end of day; in a time-only column 00:00 would read as start " + "of day, so this is held for review" +) + + +def _shift_frame() -> pd.DataFrame: + return pd.DataFrame( + { + "shift_start": ["14:00", "15:00", "13:30", "16:00", "12:00"], + "shift_end": ["22:00", "23:00", "21:30", "24:00", "20:00"], + } + ) + + +def _time_actions(report: fd.CleanReport, column: str) -> list[fd.Action]: + return [ + a + for a in report.actions + if a.step == "semantic" + and a.column == column + and a.metadata.get("expert") == "time_canonical" + ] + + +@pytest.mark.parametrize("mode", ["auto", "review", "assist"]) +def test_issue_305_repro_keeps_24_00_and_suggests(mode: str) -> None: + df = _shift_frame() + out, report = fd.clean(df, semantic_mode=mode, return_report=True, verbose=False) + + assert out["shift_end"].iloc[3] == "24:00" + assert out["shift_end"].tolist() == df["shift_end"].tolist() + actions = _time_actions(report, "shift_end") + assert [a.status for a in actions] == ["suggested"] + assert actions[0].rationale == _RATIONALE + assert actions[0].metadata.get("raw_value") == "24:00" + + +@pytest.mark.parametrize("raw, proposed", [("24:00", "00:00"), ("24:00:00", "00:00:00")]) +def test_proposal_scores_between_review_and_auto_thresholds(raw: str, proposed: str) -> None: + df = pd.DataFrame({"end_time": ["08:00", "09:30", "10:15", "11:00", raw]}) + config = CleanConfig(semantic_mode="auto") + ctx = build_semantic_context(df, config) + info = ctx.columns["end_time"] + expert = TimeCanonicalExpert() + assert expert.applies(info) + + proposals = expert.propose(df["end_time"], info) + assert len(proposals) == 1 + proposal = proposals[0] + assert proposal.raw_value == raw + assert proposal.proposed_value == proposed + assert config.semantic_review_threshold <= proposal.confidence + assert proposal.confidence < config.semantic_auto_threshold + assert proposal.rationale == _RATIONALE + assert "instant is unchanged" not in proposal.rationale + + +def test_seconds_form_is_kept_in_auto_mode() -> None: + df = pd.DataFrame({"end_time": ["08:00:00", "09:30:00", "10:15:00", "11:00:00", "24:00:00"]}) + out, report = fd.clean(df, semantic_mode="auto", return_report=True, verbose=False) + assert out["end_time"].iloc[4] == "24:00:00" + assert [a.status for a in _time_actions(report, "end_time")] == ["suggested"] diff --git a/tests/truthbench/test_fixtures.py b/tests/truthbench/test_fixtures.py index c8c25a55..3d25fe9f 100644 --- a/tests/truthbench/test_fixtures.py +++ b/tests/truthbench/test_fixtures.py @@ -344,8 +344,18 @@ def test_logistics_content_families_are_labeled_on_actual_cells() -> None: "transport_time", "twentyfour-hour-transport", "24:00", + Disposition.REVIEW, + ) + spaced = _assert_cell( + fixture, + "log-07", + "tracking_status", + "spaced-tracking-status", + "on time", Disposition.REPAIR, ) + assert spaced.expected_output is not None + assert spaced.expected_output.value == "on-time" for row_id, column, family in ( ("log-09", "address", "address-pii"), ("log-15", "tracking_status", "late-tracking-canary"), @@ -562,6 +572,7 @@ def test_eight_domain_corpus_contains_required_trap_categories() -> None: "temperature-unit-f", "cross-timezone-window", "twentyfour-hour-transport", + "spaced-tracking-status", "address-pii", "late-tracking-canary", "protected-shipment-id-conflict",