test(streaming,memory): batch-boundary invariance and memory-replay collision safety - #481
Merged
Merged
Conversation
…ay collisions
Phase 15/16 of the production-readiness campaign. Tests only, no source change.
tests/test_streaming_boundaries.py feeds the same rows through StreamingCleaner
as 1/10/100/1000 batches and separates what is batch-independent (row count,
schema, reservoir quantiles, min/max, unsaturated Space-Saving mode, missing
accounting — all bit-identical; Welford mean/std to ~1 ULP) from what is not
(which value fills a gap, how many gaps fill at all, saturated Space-Saving,
recent-window dedup, drift-event count). It also covers duplicates across batch
boundaries, columns appearing and disappearing mid-stream, 1-row and single-huge
batches, and measures the structures behind the constant-memory claim.
tests/test_memory_conflicts.py covers the categorical one-token replay trap
("M -> male" learned on one dataset, replayed onto one where M means medium),
which test_cleaning_memory.py covers only for dates, plus same-value/different
column, same-value/different semantic type, stale memory, dataset-identity
collisions, corrupted and foreign payloads, and audit-trail distinguishability.
Tests whose docstring starts with FINDING pin current behaviour reported as a
defect or specification gap, not as desired behaviour:
- memory and profile replay auto-apply a one-token categorical repair onto a
same-named column whose meaning changed (S2)
- StreamingState.drift_log and StreamingCleaner._exception_batches grow without
bound; StreamingState.columns grows with distinct column names (S2/S3)
- a "never modify" constraint is inert for a column absent from batch 1 (S2)
- an empty micro-batch scores 0.0 trust and fails fail_under_trust (S3)
- global_duplicates=True is recent-window only and dedups nothing in one batch
- a non-numeric threshold in a memory file crashes fd.clean with a bare
TypeError; the SQLite store silently overwrites on dataset_id collision (S3)
- CleaningMemory has no schema version and freshdata_version is never read (S4)
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 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 |
kevincostner17
added a commit
that referenced
this pull request
Sep 20, 2026
…lands on (#489) A repair learned on one dataset was replayed onto another whose column meant something different, and applied automatically: A: segment = M/F -> learns M -> male B: segment = S/M/L -> becomes S/male/L/male (status=automatic, human_review=False, confidence 0.95) Clothing sizes silently rewritten as genders. The conflict machinery already existed and worked: _conflict_proposal turns a deterministic-vs-memory clash into a high-risk unsafe_ambiguous record that is never auto-applied. But it only fires on disagreement. On B the CategorySynonymExpert correctly abstains -- the column is not gender-like -- and an abstention is not a disagreement, so _merge_proposals passed the replayed repair through carrying the full confidence it earned elsewhere: det_list = det_by_key.get(key) if not det_list: merged.append(mem_p) # no deterministic proposal at all continue Every other branch was careful. Only total silence had no handling, and silence is exactly what happens when the evidence behind a learned repair is absent. The stored column_signature/value_signature would not have caught it: both columns profile as role="categorical", semantic_type=None, free_text=False. A replayed repair whose issue type is context-dependent -- category_synonym, boolean_synonym, reference_value, where the same token means different things in different columns -- is now demoted to a review-required suggestion when nothing in this frame corroborates it. Context-free repairs (encoding_repair, format_alignment, numeric_format and the rest) are untouched: they mean the same thing in every column, so demoting them would cost the feature without buying any safety. sizes + gender memory S/M/L kept, suggested / risk=high / human_review genders + gender memory still auto-applied -- the feature still works Provenance survives the demotion, which matters more for a held decision than an applied one: memory_influenced, model_id ending :memory, backend="memory", evidence kinds memory_replay and a new memory_uncorroborated note carrying the reason. 4 of the 5 new tests fail on main; the one that passes is the feature-still-works control. This also flips five tests that #481 pinned as current behaviour when it found the defect. They were tripwires for the fix and the fix tripped them; each now asserts the repaired behaviour with its history kept in the docstring. Two findings in that file stay pinned because they remain true: freshdata_version is written and never read, and the SQLite store overwrites on a dataset_id collision. Full suite 7098 passed / 0 failed, coverage 94.94%; ruff clean repo-wide.
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
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.
Production-readiness lane: Phase 15 (streaming batch-boundary invariance) and
Phase 16 (learning/memory replay safety). Tests only — no source changes.
Two new files, 55 tests:
tests/test_streaming_boundaries.py(33) — feeds the same rows throughStreamingCleaneras 1 / 10 / 100 / 1000 batches and pins down exactly what isbatch-independent and what is not, plus duplicates across boundaries, schema
changes mid-stream, pathological batch sizes, and the constant-memory claim.
tests/test_memory_conflicts.py(22) — the categorical one-token replay trap(
M -> malelearned on dataset A, replayed onto a dataset B whereMmeansmedium) and the collision axes around it.
tests/test_cleaning_memory.py:237already covers the DATE version of a memory/deterministic disagreement; the
categorical case did not exist.
Tests whose docstring starts with
FINDINGpin current behaviour that thislane reports as a defect or a specification gap, not as desired behaviour.
What IS batch-invariant
totalmissing/non_null,finalize()accountingWhat is NOT batch-invariant (characterised, not defects)
Which value lands in a missing cell (imputation reads the running state as of
that batch); how many cells get imputed at all (warmup deferral + the cumulative
missing-band gate both move with batch size, and warmup-deferred cells are never
back-filled); the Space-Saving mode once
max_categoriessaturates; recent-windowcross-batch dedup; the drift-event count.
Findings
M -> malelearned on a gender column rewritesS/M/Lsizes at confidence 0.95,risk="medium",human_review=False. The deterministic expert correctly abstains there (gender_likeis false), but an abstention is not a disagreement, so_merge_proposalspasses the memory proposal straight through. Normalized matching also catches a dominant lowercasem. Reproduces throughprofile=value maps (semantic:allowed_value_map:profile), so the hole is in replay, not inCleaningMemory. The storedcolumn_signature/value_signatureare never compared at retrieval and would not have helped (both frames profile identically); what is missing is a re-check that the evidence behind the learned repair still holds.StreamingState.drift_log(_state.py:224) is an uncapped list. On a persistently drifting stream it grows strictly linearly: 50 batches → 74 entries / 23 KB; 4000 batches → 5999 entries / 1.85 MB (~310 B/entry). Proposed fix: back it withdeque(maxlen=...)exactly like_trust_history(cap10_000). Bothstate_["n_drift_events"]andfinalize().streaming["drift_events"]are currently derived fromlen(drift_log), andfinalize()replays one warning per retained entry, so a cap needs a separate monotonic total beside it and should disclose how many entries it dropped.StreamingCleaner._exception_batches(_cleaner.py:93) retains the full DataFrame of every quarantined row for the whole stream: 20 batches → 361 rows / 29 KB; 400 batches → 7581 rows / 614 KB. A 100M-row stream with a few percent late data holds millions of rows in memory — the largest hole in "feeding 100M rows costs the same memory as feeding 100k". Proposed fix: amax_retained_exceptionscap onStreamingCleanConfigand/or a spill-to-parquet sink, keepinglate_quarantined_totalexact;last_exceptions_already covers the per-batch case.context="Never modify bonus."is silently inert whenbonusfirst appears in a later batch — the streaming imputer then writes into it. The only signal is a batch-1 unresolved-reference warning; batch 2 says nothing.docs/streaming.mddocuments the converse ("a batch that happens to be missing a column can't make the policy drift") but not this direction.batch_trust_score == 0.0, failsfail_under_trust, counts as a gate failure (non-zero CLI exit) and drags the unweighted rolling trust down. Only the row-weighted cumulative score ignores it.StreamingState.columns/StreamingCleaner._rolesare keyed by column name with no eviction, and each entry owns a full reservoir (20k floats by default) plus a counter. A schema-churning stream (partition- or date-named columns) grows state linearly in distinct column names.global_duplicates=Truecompares a row only against earlier batches. A single batch de-duplicates nothing (12 identical-pattern rows → 12 out), and the surviving row count is a function of how the caller chunked the stream (2 batches → 6, 4 → 3, 12 → 3). The name reads as a global guarantee.config_overrides()copies any value stored under an allowed threshold key straight intoCleanConfigunvalidated. A memory holding{"missing_threshold_low": "not-a-number"}takes downfd.cleanwith a bareTypeError: '<' not supported between instances of 'float' and 'str', naming neither the memory nor the field.dataset_idalone withON CONFLICT DO UPDATE. Saving a memory for an unrelated schema under an id already present destroys the previous memory — different signature hash, different learned repairs — with no warning and no recovery. The plain-JSON path does guard identity.semantic_typedoes not block a colliding replay. Onlyallowed_values,mutable=False, ortarget/id/preserveprotection do.CleaningMemoryJSON carries onlyfreshdata_version, which is written on learn and never read again — an ancient or future memory replays exactly like a current one.from_dictis a barepayload.get(...)per field with no type checking, so a foreign JSON document loads as an empty-but-valid memory (acceptedstays astr). It is caught downstream bymatch(), so nothing is corrupted, but the diagnostic blames data drift rather than a bad file. Contrastfreshdata.learning, which shipsProfileManifest/ProfileVersionErrorfor exactly this.freshdata.streaming._timeseries.to_timedelta("1s")emits a numpy ≥ 2.5DeprecationWarning("generic unit for NumPy timedelta"), which will become an error. CI does not see it:filterwarningsonly escalates warnings whose module isfreshdata, and this one is raised inside pandas.Verified safe (suspected, disproved)
totalstays exact even after.memory_influenced, the policy status,model_idsuffix:memory,SemanticEvidence(kind="memory_replay"),backend="memory"), and survivereport.to_dict()→ JSON. Note the status nuance: a semantic replay carries the policy-gate status ("automatic"), whilestatus="approved"is whatannotate_reportstamps on replayed non-semantic decisions and its ownstep="memory"summary.match()fails and retrieval is keyed on the column name).M -> mvs memoryM -> maleproduces oneunsafe_ambiguousrecord,risk="high",status="suggested",human_review=True, value never mutated.match()refuses it with an explanatory warning.