Skip to content

TSC preflight parity: sequence_id null set uses coercion sentinels, not pandas' default NA — 'none' ids over-reject, '#NA' under-rejects #239

Description

@LukasWodka

Same family as #218, but a different validator and not covered by that fix.

What

The time_series_classification local dry-run preflight decides whether a sequence_id cell is null in CheckSequenceRows / sequenceScanFrom (internal/push/preflight.go) by trimming the cell and testing membership in the package-level naSentinels map (the curated coercion.NA_SENTINELS set, which includes lowercase "none").

But that check previews the ingestor's SequenceGroupValidator (tracebloc/data-ingestors tracebloc_ingestor/validators/sequence_group_validator.py), which plain-reads the CSV (pd.read_csv, keep_default_na=True) and computes null as:

ids = df[resolved]
null_mask = ids.isna() | (ids.astype(str).str.strip() == "")

So the faithful null set is pandas' default STR_NA_VALUES ∪ {whitespace-only strings}, matched on the raw cell (pandas tokenises NA before stripping) — NOT the curated coercion set. Two concrete divergences result:

  • "none" (lowercase) → false REJECT (over-reject, the dangerous direction). pandas keeps "none" (it is not in STR_NA_VALUES), so SequenceGroupValidator accepts. The CLI treats "none" as null (it is in naSentinels) → it rejects a dataset the cluster ingests fine, blocking the ingest before it starts. Verified: driving the real SequenceGroupValidator (pinned data-ingestors ref) on a CSV with none sequence ids returns is_valid=True.
  • "#NA" → under-reject. pandas drops "#NA" to NaN (it is in STR_NA_VALUES) → the ingestor rejects it as a null id. The CLI's coercion set lacks "#NA" → the CLI misses it and would only find out in-cluster.

Padded sentinels diverge too (e.g. " NA "): the ingestor tokenises NA on the raw field, so a padded token is a real id in-cluster; the CLI's pre-strip makes it null → over-reject.

Cost

A customer whose TSC data uses a sequence id that happens to be the string "none" (realistic — a placeholder/label leaking into the group key) has their ingest blocked locally even though the cluster would accept it — the inverse of what the dry-run is for. The "#NA" direction is the milder under-reject (a burned upload).

Fix

#218 already added a pandasDefaultNA map (verbatim pandas._libs.parsers.STR_NA_VALUES at the pinned pandas 3.0.3) to internal/push/preflight.go for the grouped label/time checks. Make CheckSequenceRows / sequenceScanFrom use it too, plus mirror the .str.strip() == "" clause for whitespace-only cells:

a sequence_id cell is null iff raw ∈ pandasDefaultNA OR strings.TrimSpace(raw) == "" — matched on the raw cell, dropping the pre-strip.

("" is already in pandasDefaultNA, so the whitespace clause only adds genuinely whitespace-only cells.) Add a parity case (tsc-none-sequence-id, accept) driven against the real SequenceGroupValidator, and confirm the existing tsc-null-sequence-id case (empty + NA ids) still rejects. Same parity discipline as the rest of the preflight-parity harness.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions