fix(validators): wire 0-record guard into tabular categories (dropped from #314 merge)#316
Closed
shujaatTracebloc wants to merge 1 commit into
Closed
fix(validators): wire 0-record guard into tabular categories (dropped from #314 merge)#316shujaatTracebloc wants to merge 1 commit into
shujaatTracebloc wants to merge 1 commit into
Conversation
…sweep follow-on) Adversarial tabular testing found a header-only / empty CSV passes preflight when NO schema is supplied: DataValidator (which catches it via "No data found to validate") is only added when a schema is present, and tabular had no IngestableRecordsValidator. IngestableRecordsValidator was explicitly designed to cover this (its docstring names "non-file-bearing (tabular / time-series) categories" with file_subdir=None) but was never wired in. Fix: add IngestableRecordsValidator(file_subdir=None) — row-count check only, no file pairing — to tabular_classification and tabular_regression, so a zero-record tabular manifest fails fast regardless of schema presence (with a schema, DataValidator still reports first). Tests: mapping (both tabular categories carry IngestableRecordsValidator with file_subdir=None); updated the prior "tabular has neither" assertion to "has the 0-record guard but not the text-content validator". Full suite: 1227 passed, 1 xfailed, coverage 96.6% (gate 95%). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Superseded by #317, which fixes the tabular 0-record gap centrally — it composes the 0-record guard (plus table-name / duplicate / label-diversity) once in map_validators for every category via ModalitySpec traits, instead of wiring it per-factory. #317 covers tabular automatically and removes the repetition that caused this gap. |
shujaatTracebloc
added a commit
that referenced
this pull request
Jun 18, 2026
…tegory (#317) The 0-record guard, table-name, duplicate, and label-diversity validators were copy-pasted across the per-category factories: TableName + Duplicate in all 11, the 0-record guard wired three different ways (NLP `_nlp_content_validators`, vision `_zero_record_validator`, and inline for tabular), and label-diversity called in each of the 6 classification factories. That repetition is what made the tabular 0-record gap possible (a factory simply forgot to add it). Centralize the universally-applicable validators in `map_validators`, driven by declarative `ModalitySpec` traits, so each factory returns ONLY its category-specific validators: [IngestableRecordsValidator(file_subdir=spec.file_subdir)] # every category + spec.build_validators(options) # category-specific + [LabelDiversityValidator] if spec.is_classification + [TableNameValidator, DuplicateValidator] # every category - New `ModalitySpec` traits: `file_subdir` (images/texts/sequences/None) and `is_classification`. - `map_validators` builds the common frame; factories drop the repeated lines. - Deleted `_zero_record_validator`; `_nlp_content_validators` collapses to the text-content check (`_text_content_validator`); `label_diversity_validator` is now public (composed by map_validators). Behavior-preserving — the resulting validator SET per category is unchanged, EXCEPT tabular_classification / tabular_regression / time_series_forecasting / time_to_event_prediction now correctly get the 0-record guard (closing the tabular gap centrally; supersedes #316). Order changes only in that the 0-record guard now leads (fail-fast) — pass/fail behavior is order-independent. Tests: updated order-sensitive mapping assertions; added `test_common_validator_frame_composed_for_every_category` locking the frame. Full suite: 1226 passed, 1 xfailed, coverage 96.6% (gate 95%).
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.
Summary
Re-applies a fix that was dropped from the #314 squash merge. The tabular 0-record guard was committed to the #314 branch but landed after the squash point, so merged
developstill lacks it — verified by re-testing the merged code: a no-schema header-only / empty tabular CSV still passes preflight ondevelop.Gap
tabular_classification/tabular_regressionaddDataValidator(which rejects an empty CSV with "No data found to validate") only when a schema is supplied. With no schema there's noDataValidatorand no 0-record guard, so a header-only CSV creates an empty table and fails late at registration.IngestableRecordsValidatorwas explicitly designed for this ("non-file-bearing tabular / time-series categories",file_subdir=None) but was never wired into the tabular factories.Fix
Add
IngestableRecordsValidator(file_subdir=None)(row-count check only, no file pairing) to both tabular factories — rejects a zero-record manifest regardless of schema (with a schema,DataValidatorstill reports first).Verification (against merged develop)
IngestableRecordsValidator, "No data rows found in CSV…")Tests: mapping assertion (both tabular categories carry
IngestableRecordsValidator(file_subdir=None)). Full suite: 1227 passed, 1 xfailed, coverage 96.6% (gate 95%).🤖 Generated with Claude Code
Note
Low Risk
Narrow preflight validation change for tabular ingest; rejects empty manifests earlier with no auth, data-model, or broad behavioral refactors.
Overview
Tabular classification and regression now run
IngestableRecordsValidator(file_subdir=None)during preflight so header-only or empty CSV manifests are rejected before ingest, even when no schema is provided.Previously,
DataValidatoronly ran with a schema, so a no-schema empty tabular CSV could pass preflight and fail later at registration. Withfile_subdir=None, the guard performs a row-count check only (no file pairing), matching the existing design for non-file-bearing categories.Tests assert both tabular categories include this validator with
file_subdir is None, and the NLP/vision mapping test docstring is updated to reflect tabular now having the guard.Reviewed by Cursor Bugbot for commit 5759e97. Bugbot is set up for automated code reviews on this repo. Configure here.