Stop re-pickling arguments that can never be pickled

src/nodrift/cli.py

@@ -194,13 +194,15 @@ def _print_not_covered(abandoned: list[str], verbose: bool) -> None:
     """Say which functions the verdict above does not speak for.
 
     Without this, `check` reports "no behaviour change" while silently
-    omitting every function whose inputs were too large to record — on
-    `sqlparse` that was 40 of the most important ones.
+    omitting every function whose inputs could not be recorded — because they
+    were too large, or because they were never picklable in the first place
+    (a function taking a callback on every call, say). On `sqlparse` that is
+    42 of the most important ones.
     """
     if not abandoned:
         return
     print(f"  {len(abandoned)} function(s) not fully recorded "
-          f"(inputs too large to capture) — not covered by this check")
+          f"(inputs too large, or not picklable) — not covered by this check")
     if verbose:
         for name in abandoned:
             print(f"      {name}")

src/nodrift/plugin.py

@@ -183,7 +183,7 @@ def pytest_unconfigure(config):
     if summary.get("abandoned"):
         print(
             f"[nodrift] {len(summary['abandoned'])} functions not recorded "
-            f"(inputs too large to capture); they are NOT covered",
+            f"(inputs too large, or not picklable); they are NOT covered",
             file=sys.stderr,
         )
     _recorder = None

src/nodrift/recorder.py

@@ -17,8 +17,6 @@
 import types
 from collections import defaultdict
 
-from .fingerprint import digest, fingerprint
-
 _local = threading.local()
 
 _GZIP_MAGIC = b"\x1f\x8b"
@@ -93,7 +91,23 @@ def __init__(
         # recording, so oversized inputs are counted and dropped.
         self.max_blob_bytes = max_blob_bytes
         self.abandon_after = 3
+        # An oversized blob is evidence about that one input; a pickling
+        # failure is usually evidence about the signature. But "usually" is
+        # not "always" — a function that takes a callback on some paths and
+        # plain data on others is still worth recording — so a failure gets
+        # considerably more rope than an oversized input before the target is
+        # written off. It costs almost nothing to be generous here: a
+        # signature that genuinely cannot be pickled fails thousands of times
+        # in a run, so it trips any threshold in this range immediately.
+        self.abandon_unpicklable_after = 16
         self._oversized_streak: dict[str, int] = defaultdict(int)
+        # Some functions take a callback — a lambda, a local function, a
+        # closure — on every single call. Those arguments can never be
+        # pickled, and the pickler only discovers that after walking the rest
+        # of the argument graph, so the full serialisation cost is paid and
+        # then thrown away. Failure is a property of the signature far more
+        # often than of one input, so it is worth remembering.
+        self._unpicklable_streak: dict[str, int] = defaultdict(int)
         self._abandoned: set[str] = set()
         # Adaptive sampling: how many consecutive duplicates before backing
         # off, and how far back-off is allowed to go.
@@ -102,7 +116,7 @@ def __init__(
         self._dup_streak: dict[str, int] = defaultdict(int)
         self._skip: dict[str, int] = defaultdict(lambda: 1)
         self._countdown: dict[str, int] = defaultdict(int)
-        self.calls: dict[str, dict[str, bytes]] = defaultdict(dict)
+        self.calls: dict[str, dict[bytes, bytes]] = defaultdict(dict)
         self.recorded_outcome: dict[str, dict[str, list]] = defaultdict(dict)
         self.stats = defaultdict(int)
         # Guards every counter below. A threaded suite would otherwise exceed
@@ -246,6 +260,10 @@ def _capture(self, target: str, args, kwargs) -> None:
             return
         with self._lock:
             if target in self._abandoned:
+                # Counted, not just skipped: every call has to land in some
+                # bucket or the totals stop adding up, and abandonment is now
+                # the common reason a call is never looked at.
+                self.stats["skipped_abandoned"] += 1
                 return
             bucket = self.calls[target]
             if len(bucket) >= self.max_per_target:
@@ -275,11 +293,26 @@ def _capture(self, target: str, args, kwargs) -> None:
         except Exception:
             with self._lock:
                 self.stats["unpicklable"] += 1
+                # Same reasoning as oversized inputs: paying the serialisation
+                # cost only to discard the result is exactly what makes a
+                # library slow to record. A target whose arguments keep
+                # refusing to pickle is not going to start, so stop asking —
+                # and say out loud that it is not covered, rather than
+                # retrying it silently for the rest of the run.
+                self._unpicklable_streak[target] += 1
+                if (self._unpicklable_streak[target]
+                        >= self.abandon_unpicklable_after):
+                    self._abandoned.add(target)
+                    self.stats["abandoned_targets"] += 1
+                    self.stats["abandoned_unpicklable"] += 1
             return
         finally:
             _local.busy = False
 
         with self._lock:
+            # The pickle succeeded, so whatever made earlier calls fail was
+            # about those inputs and not about this function.
+            self._unpicklable_streak[target] = 0
             if len(blob) > self.max_blob_bytes:
                 self.stats["oversized"] += 1
                 # Paying the pickle cost only to discard the result is what
@@ -293,8 +326,11 @@ def _capture(self, target: str, args, kwargs) -> None:
             self._oversized_streak[target] = 0
 
             self.stats["captured"] += 1
-            key = digest(["blob", len(blob), _cheap_hash(blob)])
-            if key in bucket:
+            # The pickled bytes are themselves the identity of the input, so
+            # they are the key. Hashing them into a digest first cost a full
+            # blake2b pass over every blob plus a JSON round-trip, to produce
+            # something no more precise than the bytes it replaced.
+            if blob in bucket:
                 # Seen before. Back off, but never so far that a function which
                 # later receives novel inputs stays invisible.
                 self._dup_streak[target] += 1
@@ -313,7 +349,7 @@ def _capture(self, target: str, args, kwargs) -> None:
             # fully again.
             self._dup_streak[target] = 0
             self._skip[target] = 1
-            bucket[key] = blob
+            bucket[blob] = blob
 
     # -- teardown -------------------------------------------------------
 
@@ -349,12 +385,6 @@ def dump(self, path: str) -> dict:
         return summary
 
 
-def _cheap_hash(blob: bytes) -> str:
-    import hashlib
-
-    return hashlib.blake2b(blob, digest_size=16).hexdigest()
-
-
 def merge_recordings(shards: list[str], out: str, cap: int | None = None) -> dict:
     """Combine per-worker recordings into one, dropping duplicates.
 

tests/test_fingerprint.py

@@ -125,6 +125,54 @@ def test_sampling_resets_when_a_function_stays_productive():
     assert recorder.stats.get("sampled_out", 0) == 0
 
 
+def test_a_function_whose_arguments_never_pickle_is_given_up_on():
+    """Unpicklable arguments must not be re-attempted for the whole run.
+
+    A function taking a lambda on every call can never be recorded, but the
+    pickler only discovers that after walking the rest of the argument graph.
+    On `sqlparse` two such functions cost 5.2s of a 10s run — more than half
+    the recording time, spent entirely on serialisation that was thrown away.
+    """
+    from nodrift.recorder import Recorder
+
+    recorder = Recorder(["_none_"], max_per_target=600)
+    attempts = 0
+    for _ in range(5000):
+        before = recorder.stats["unpicklable"]
+        recorder._capture("m:cb", (lambda t: t, "payload"), {})
+        attempts += recorder.stats["unpicklable"] - before
+
+    assert "m:cb" in recorder._abandoned
+    assert attempts <= recorder.abandon_unpicklable_after, (
+        f"{attempts} wasted pickles; should stop at "
+        f"{recorder.abandon_unpicklable_after}"
+    )
+    # The gap has to be declared as a gap, not merely skipped quietly.
+    assert recorder.stats["abandoned_unpicklable"] == 1
+
+
+def test_a_function_that_only_sometimes_takes_a_callback_stays_recorded():
+    """Giving up must be about the signature, not one awkward input.
+
+    This is the coverage half of the trade: on `sqlparse`, abandoning after 3
+    consecutive failures lost 66 inputs from two filters that take a callback
+    on some paths and plain data on others. Nothing is gained by writing those
+    functions off, because a truly unpicklable one fails thousands of times.
+    """
+    from nodrift.recorder import Recorder
+
+    recorder = Recorder(["_none_"], max_per_target=600)
+    for i in range(300):
+        # A short burst of failures, then real inputs — never long enough in a
+        # row to look like a signature that cannot be pickled.
+        recorder._capture("m:mixed", (lambda t: t,), {})
+        recorder._capture("m:mixed", (lambda t: t,), {})
+        recorder._capture("m:mixed", (i,), {})
+
+    assert "m:mixed" not in recorder._abandoned
+    assert len(recorder.calls["m:mixed"]) > 100, "novel inputs were lost"
+
+
 def test_the_cap_holds_when_threads_record_at_once():
     """A threaded suite must not be able to record past max_per_target.
 
@@ -181,6 +229,7 @@ def test_stats_add_up_when_threads_record_at_once():
         + recorder.stats["skipped_at_cap"]
         + recorder.stats["unpicklable"]
         + recorder.stats["oversized"]
+        + recorder.stats["skipped_abandoned"]
     )
     assert accounted == total, f"{total - accounted} calls unaccounted for"
     assert len(recorder.calls["m:i"]) == recorder.stats["captured"]