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 |
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.