fix(preflight): TSC group-integrity parity — mirror di#359 label-constant + time-order validators (#218)#234
Open
LukasWodka wants to merge 1 commit into
Open
fix(preflight): TSC group-integrity parity — mirror di#359 label-constant + time-order validators (#218)#234LukasWodka wants to merge 1 commit into
LukasWodka wants to merge 1 commit into
Conversation
…cy + per-group time order)
The time_series_classification local preflight only checked column presence
(CheckSequenceSchemaColumns) and null/empty sequence_id rows (CheckSequenceRows).
It did NOT mirror the two grouped validators di#359 (WS1) added, so a
mid-sequence label flip or per-group unsorted timestamps passed the local
dry-run and were rejected only in-cluster after the upload.
Add CheckTSCGroupIntegrity — one pass over the CSV grouped by sequence_id,
previewing both in the ingestor's factory order:
- labelConstantViolation ≙ LabelConstantWithinGroupValidator: one label per
sequence_id. Groups drop null ids (dropna=True), a null/empty label counts
as its own value (nunique(dropna=False)), and numeric labels collapse only
when the whole column is numeric ("1"/"1.0" are not a false flip).
- perGroupTimeViolation ≙ PerGroupTimeOrderedValidator: monotonic non-
decreasing per sequence (ties allowed; interleaving fine). Faithfully mirrors
the NUMERIC branch (pd.to_numeric coerce; invalid/NA → reject). Deliberately
does NOT preview the TIMESTAMP/date branch — reproducing pandas' mixed-format
parse + locale-ambiguity guard in Go would risk over-rejecting a date the
cluster accepts (the dangerous direction). That date-typed ordering stays a
documented under-preview.
Fix an NA-sentinel parity over-reject (review): the grouped validators plain-
read_csv (keep_default_na=True), so their label-numeric-collapse and
sequence-id dropna see pandas' GLOBAL default NA set (STR_NA_VALUES), NOT the
curated coercion.NA_SENTINELS that naSentinels mirrors (used only by the
label-diversity preview, which pins na_values to that set). Mirroring the
curated set here OVER-rejected: a label column pandas reads as all-numeric
(a default sentinel like "#NA" collapsing to NaN, so "1"/"1.0" don't flip) was
seen by the CLI as a mixed object column and flagged as a false mid-sequence
flip. Add pandasDefaultNA (verbatim STR_NA_VALUES at the pinned pandas 3.0.3)
and use it for the grouped checks. Also stop trimming cells in scanTSCRows —
pandas keeps whitespace on object columns / groupby keys and matches NA on the
raw cell; the numeric-collapse and numeric-time paths trim only inside the
parse attempt, mirroring pandas' numeric coercion (which does tolerate " 1").
The float64 collapse still under-approximates pure-int64 columns past 2^53
(safe under-reject; documented).
Strictly grouped-scoped: the sole caller gates on GroupingFor's grouping trait,
so no non-grouped category runs this — important because TSC is develop-ahead
(di#359 is NOT in the deployed v0.7.0 ingestor).
Flip cases.json tsc-label-flip / tsc-unsorted-timestamp from the documented
accept/reject gap to reject/reject (Go preview now mirrors); add
tsc-numeric-label-na-sentinel (accept/accept) pinning the over-reject fix
against the REAL validator; goldens regenerated from the pinned #359 ref. Add
TestCheckTSCGroupIntegrity (14 boundary cases, incl. the pandas-default-NA
sentinel accept and a padded-object-label flip) and verify
sync-validator-goldens.sh --check passes.
Closes #218
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
What
Closes #218. The CLI's
time_series_classificationlocal preflight previewed only column-presence + null-sequence_idrows — so a mid-sequence label flip or per-group unsorted timestamps passed the local dry-run but were rejected in-cluster. This mirrors di#359's two TSC group validators as a contract-locked preview (Principle 6: the ingestor owns the logic; the CLI mirrors it under the parity harness).What it adds
CheckTSCGroupIntegrity(internal/push/preflight.go) — one pass grouped bysequence_id, previewing:LabelConstantWithinGroupValidator): one label per sequence. Null-id rows dropped (pandasdropna); a null label counts as its own value; numeric-label collapse only when every non-null cell is numeric (so"1"/"1.0"isn't a false flip); single-row / already-constant groups never flagged.PerGroupTimeOrderedValidator): monotonic non-decreasing per sequence (ties OK, interleaving OK). The numeric branch is mirrored (NA/unparseable → reject); the TIMESTAMP/date branch is intentionally a documented safe under-preview (accept) — reproducing pandas' permissive, locale-ambiguous date parse in Go would risk over-rejecting a date the cluster accepts.Strictly TSC-scoped: the sole caller gates on the vendored layout contract's grouping trait, so no non-grouped / non-TSC dataset is touched.
Parity + deployment safety
.data-ingestors-refalready pins the di#359 merge (7b4ecac); goldens fortsc-label-flip/tsc-unsorted-timestamp/ the newtsc-numeric-label-na-sentinelcase are generated by driving the real Python validators (not hand-authored).sync-validator-goldens.sh --checkis green.pd.read_csv(pandas defaultna_values), so the checks use pandas' exact defaultSTR_NA_VALUES(19 tokens), not the CLI's curated coercion set — otherwise a numeric label column pandas NA's out would be a false flip (over-reject). Whitespace handled faithfully (raw cells; trim only inside the numeric parse); numeric collapse usesfloat64(documented int64>2⁵³ edge — under-rejects, the safe direction).Tests
TestValidatorParitygreen incl. label-flip / unsorted-timestamp (reject) + the NA-sentinel case (accept).TestCheckTSCGroupIntegrity(single-row, ties, interleaving, numeric collapse, null-label flip, invalid numeric time, null-id drop, date-branch benign skip, padded-object flip, NA-sentinel accept).<→<=over-strict → ties test fails; NA-set revert → over-reject reproduced).go build/vet/gofmt/go test/coverage-floorall green.Follow-up (out of scope, flagged)
CheckSequenceRows/sequenceScanFrom(pre-existing) has the same NA-set bug family previewingSequenceGroupValidator(another plain-read_csvpath): asequence_idliterally"none"is rejected by the CLI but accepted by the ingestor (over-reject), and"#NA"is under-rejected. Filed separately to keep this PR scoped.Closes #218.
🤖 Generated with Claude Code
Note
Medium Risk
New grouped TSC validation mirrors pandas/ingestor semantics (NA tokens, dtype collapse, raw whitespace) and is parity-locked, but mistakes could still false-reject or miss issues; scope is limited to grouped TSC preflight only.
Overview
Adds local preflight for grouped
time_series_classificationso mid-sequence label changes and per-sequence unsorted timestamps fail the dry-run the same way the in-cluster di#359 validators do, instead of only after upload.CheckTSCGroupIntegrity(single CSV pass viascanTSCRows) runs after nullsequence_idchecks inPreflightDatasetwhen the category has the grouping trait. It previews one label per sequence (pandasgroupbywith null ids dropped, numeric label collapse when the column is all-numeric, raw cells for NA/object parity) and monotonic non-decreasing time within each sequence for schema-declared numeric time columns (ties and interleaved sequences allowed). TIMESTAMP/date ordering is intentionally not previewed to avoid over-rejecting vs pandas.Grouped checks use
pandasDefaultNA(STR_NA_VALUES), not the curatednaSentinelsused for schema-typed label diversity — fixing false rejects when#NAmakes the label column numeric in-cluster.Parity harness updates:
tsc-label-flipandtsc-unsorted-timestampnow reject locally; newtsc-numeric-label-na-sentinelaccept case;TestCheckTSCGroupIntegritytable tests the accept/reject boundaries.Reviewed by Cursor Bugbot for commit fe46ff0. Bugbot is set up for automated code reviews on this repo. Configure here.