fix(semantic): suggest instead of auto-applying 24:00 -> 00:00 in time-only columns (#398)

TimeCanonicalExpert proposed '24:00' -> '00:00' at base_confidence 0.96,
above the 0.95 auto threshold, so semantic_mode='auto' and 'review' both
applied it. Its rationale, "the instant is unchanged", only holds when a
date travels with the value: 24:00 on day D is 00:00 on day D+1. In a
column of clock times alone the rewrite turns end-of-day into
start-of-day, so a shift_end of 24:00 sorted before its shift_start.

The proposal now scores 0.80, between the review (0.70) and auto (0.95)
thresholds, so it is recorded as a suggestion and routed to a human in
every mode. The rationale and docstring say why it is held for review.

Suggestions are never learned into cleaning memory, so the replay case
for time_canonical is replaced by a test that nothing is learned or
replayed. The TruthBench logistics oracle for log-06 transport_time moves
from REPAIR to REVIEW. That was the domain's only REPAIR cell and every
domain must cover all dispositions, so log-07 tracking_status 'on time'
-> 'on-time' is added as the domain's exact repair.

Closes #305

Author: kevincostner17

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",

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,
                 )

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]

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"]

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",