Skip to content

fix(data ingest): reclaim the staged source copy after a clean success (#166) - #167

Merged
saadqbal merged 1 commit into
developfrom
feat/ingest-ux-sweep
Jul 7, 2026
Merged

fix(data ingest): reclaim the staged source copy after a clean success (#166)#167
saadqbal merged 1 commit into
developfrom
feat/ingest-ux-sweep

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #166 (found by the ingest-flow UX audit): after a successful tracebloc data ingest, the CLI-staged source copy at SharedRoot/.tracebloc-staging/<table> was never reclaimed, so every successful push of a file-bearing dataset silently left ~2× the dataset on the shared PVC until a data delete / --overwrite.

Root cause, verified end-to-end: the CLI streams a full copy to the staging prefix; the in-cluster ingestor copies it (shutil.copy, data-ingestors/.../file_transfer.py:61 — never a move) into the final table dir. Both copies then survive; only push.Teardown (on delete/overwrite) ever removed the staging one.

What changed

  • push.CleanStaging — best-effort rm -rf of onlyStagedPrefix(table) (never the final table dir, never the MySQL table) via the same ephemeral stage-identity pod Teardown uses (owns the uid-65532 staging files → rm works on hostPath + CSI).
  • runDataIngest calls it after a clean success only (classifyPushOutcome == "succeeded"). It never fires on --detach (Job still reading the source), completed_with_failures, or any failure. A failed reclaim logs a warning and does not fail the ingest (exit stays 0, --output-json status untouched).

Safety (this rm runs on a shared multi-tenant PVC)

A 10-agent adversarial review confirmed the load-bearing properties hold — path is the exact .tracebloc-staging/<table> dir (no glob, no reg/reg_train prefix collision), ValidateTableName runs before staging, and the gate maps to exactly JobOutcomeSucceeded-with-no-failures. Three low-severity refinements from that review are folded in:

  • reclaim wait+exec bounded by StagingCleanupTimeout (45s) so a stuck cleanup pod can't add the full pod-ready timeout to an already-successful command;
  • cleanup pod created under a detached context so a parent-ctx cancel in the create window can't orphan a server-committed pod;
  • --output-json result emitted before the reclaim, so scripted consumers aren't delayed by cleanup.

Tests

TestCleanStaging_RemovesOnlyStagingPrefix (rm targets only the staging prefix, in the ephemeral stage pod, no leak) and TestCleanStaging_PodCreateFailureReturnsError (surfaced as a non-fatal error). TestClassifyPushOutcome already pins that only the clean case yields "succeeded", protecting the gate. Full internal/push + internal/cli suites green; gofmt/go vet clean.

Follow-up

The cleaner long-term fix is server-side (ingestor move-not-copy or rm its SRC_PATH after a verified load), removing the need for the extra pod — I'll file a data-ingestors ticket. This CLI-side reclaim is the immediately shippable fix for existing clusters.

Part of the tracebloc data ingest UX sweep (epic #67). Remaining audit items (silent-wait progress, undisclosed 1h watch cap, kubectl-jargon in errors, flag rejection #77) will follow as separate PRs.

Closes#166.

🤖 Generated with Claude Code

@LukasWodka@claude
fix(data ingest): reclaim the staged source copy after a clean success (
b6992a4
#166)
The CLI streams a full copy of the dataset into
SharedRoot/.tracebloc-staging/<table>, and the in-cluster ingestor
COPIES (shutil.copy, never moves) those files into the final table dir.
So after a successful `tracebloc data ingest` BOTH copies lived on the
shared PVC — the staged source was only ever removed by `data delete`
or an `--overwrite` re-ingest. Every successful push of a file-bearing
dataset silently doubled PVC usage, eventually surfacing as
"no space left on device" on a later ingest with no signal as to why.
Add push.CleanStaging: a best-effort rm -rf of ONLY StagedPrefix(table)
via the same ephemeral stage-identity pod Teardown already uses (it runs
as the uid that wrote the staging files, so the rm works by ownership on
hostPath and CSI alike). It never touches the final table dir or the
MySQL table. runDataIngest calls it after a CLEAN success only
(classifyPushOutcome == "succeeded"), so it never fires on --detach (the
Job is still reading the source), completed_with_failures, or any
failure. A failed reclaim logs a warning and does not fail the ingest.
Robustness (from pre-PR adversarial review, all low-severity):
- the reclaim's wait+exec is bounded by StagingCleanupTimeout (45s) so a
stuck/unschedulable cleanup pod can't tack the full pod-ready timeout
onto a command the user already saw succeed;
- the cleanup pod is created under a detached context so a parent-ctx
cancel in the create window can't orphan a server-committed pod;
- with --output-json the result object is emitted BEFORE the reclaim, so
scripted consumers get their result at ingest-completion latency.
Longer term the cleaner fix is server-side (ingestor move-not-copy /
remove SRC after a verified load) — will file a data-ingestors follow-up.
Closes#166. Refs #67.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@LukasWodkaLukasWodka self-assigned this Jul 7, 2026
@LukasWodka
LukasWodka requested a review from saadqbalJuly 7, 2026 13:25
@LukasWodkaLukasWodka mentioned this pull request Jul 7, 2026
Closed
@saadqbal
saadqbal merged commit 6cfb99e into developJul 7, 2026
17 checks passed
This was referenced Jul 7, 2026
Merged
Closed
@LukasWodka
LukasWodka deleted the feat/ingest-ux-sweep branch July 9, 2026 11:41
@LukasWodkaLukasWodka mentioned this pull request Jul 21, 2026
Merged
saadqbal added a commit to tracebloc/data-ingestors that referenced this pull request Jul 21, 2026
@saadqbal@divyasinghds
@claude@LukasWodka
master - release (#383)
470bf2a
* feat(ingest): emit per-column feature_stats on the global-metadata channel (#360)
At ingest, CSVIngestor now accumulates the five additive sufficient
statistics per numeric feature column — count, sum, sum_sq, min, max —
during the existing per-chunk cast pass (no second read) and emits them
as meta_data['feature_stats']. The backend folds these into global
mean/std/min/max at combine time for federated normalization (backend#1037);
only aggregates cross the cluster boundary, never raw values.
Producer side of backend#1053 / #924 G4a, slice 1.
- csv_ingestor.py: _accumulate_feature_stats folded into the INT and
FLOAT/DECIMAL cast branches; feature_stats() finalizer. Excludes the
label/target, row-id, and annotation columns — the regression target is
bucketed precisely so its raw min/max must never leak. Nulls dropped;
all-null columns omitted; sum_sq computed in float64 to avoid Int64
overflow.
- base.py: format-agnostic _collect_run_metadata() hook (default {}),
merged into file_options just before send_ingest_summary, alongside
the existing text_profile injection.
- tests/test_feature_stats.py: accumulation correctness across chunks,
feature-only exclusion, null handling, and emit wiring.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(ingest): nest feature_stats under the attributes namespace (#360)
Design review on backend#1037 finalized the dataset_meta shape: `schema`
stays a plain {column: dtype} map and every per-column extra lives under
`attributes.feature_stats`. Update the emit to match:
- CSVIngestor._collect_run_metadata now returns
{"attributes": {"feature_stats": …}} instead of top-level feature_stats.
- base.py: extract _apply_run_metadata(), which merges INTO the shared
`attributes` namespace rather than replacing it — so the per-category
attributes slice (text/image facts) can coexist with feature_stats.
- tests: assert the nested shape and the attributes-preserving merge.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(ingest): emit target-column stats for regression-class tasks (#360)
Federated target normalization for time-series forecasting (and the other
regression-class tasks) was blocked: slice 1 excluded the label column from
feature_stats, so the backend had no target stats to fold into global
mean/std/min/max (backend#1037).
Include the label/target column in feature_stats for REGRESSION_CLASS_CATEGORIES
(time_series_forecasting, time_to_event_prediction, tabular_regression); keep
excluding it for classification (its value is a class, not a numeric feature).
Row-id and annotation columns stay excluded for every category.
Scope: sufficient stats only, which unblocks Standard/MinMax/MaxAbs global
scaling. Robust/Quantile/Power need medians/quantiles/λ that aren't derivable
from these aggregates — a separate follow-up. Raw per-row target values remain
governed by label_policy="bucket"; only the additive aggregates ship, and the
target's min/max disclose its two extremes (an accepted trade, for backend#1053
sign-off).
- csv_ingestor.py: build _feature_stats_excluded from REGRESSION_CLASS_CATEGORIES
(reuses the single source of truth in cli.conventions; no cycle — cli/__init__
is import-free).
- tests: split exclusion coverage into id/annotation-always, label-excluded-for-
classification, and target-included-for-regression-class (parametrized over the
three regression-class categories).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(ingest): emit auto-detected scalar attributes for combine-time alignment (#360)
Extends the attributes tier the backend folds into the merged dataset (G4a /
backend#1037) and the edge consumes (tracebloc-engine#455/#459), with the facts
the ingestor can derive from the data — no new user-facing config:
- Categorical union vocab: the VARCHAR/CHAR/TEXT cast pass accumulates each
categorical feature column's distinct value-set (capped at
_MAX_CATEGORICAL_CARDINALITY; free-text/id-like columns dropped), emitted as
attributes.feature_stats[col].categories — the stable cross-client vocab the
edge encodes against. Label/id/annotation columns excluded.
- Temporal (time_series_forecasting): the timestamp cast pass captures the
timezone (when tz-aware) and a bounded in-order sample; sampling_frequency is
inferred via pd.infer_freq. Both omitted when unavailable.
- Image resolution: emitted as [height, width] from the run's uniform
target_size (the resolution validator enforces it) — no image read.
- Text encoding: 'utf-8' for NLP categories (non-UTF-8 text is rejected at
validation, so this is the true canonical value).
Resolution/encoding live in a format-gated BaseIngestor._scalar_attribute_metadata
so manifest-based image/text datasets emit them regardless of CSV/JSON ingestor;
CSVIngestor folds them in via super() alongside feature_stats + temporal.
Tests: categorical vocab (sorted/dedup/cross-chunk/exclusions/cap/nulls,
coexists with numeric); temporal timezone + frequency (aware/naive, gated to
time-series); image resolution [h,w]; text encoding. 26 pass; full suite green
(603 passed).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(ingest): emit user-declared color_mode/channels/bit_depth for vision alignment (#360)
Completes the image attribute tier. color_mode is user-provided in file_options
for vision use cases (not auto-detected — PIL modes like RGBA/CMYK aren't in the
contract enum) and restricted to RGB/grayscale, the values the backend G4a
contract and the edge preprocessor accept. channels derives from color_mode
(RGB->3, grayscale->1); bit_depth (8/16) is optional.
- constants.canonical_color_mode() canonicalizes user input (rgb/grayscale/grey/
gray/l, case-insensitive) to 'RGB'/'grayscale', with COLOR_MODE_CHANNELS.
- BaseIngestor._scalar_attribute_metadata emits color_mode + derived channels +
bit_depth from file_options for image datasets; a non-canonical value is
skipped (never shipped) rather than crashing.
- conventions.resolve() bridges top-level color_mode/bit_depth into file_options
(spec > top-level precedence, like target_size) and validates them at config
time — a bad value fails fast with a clear message.
Tests: emission (RGB/grayscale channels, bit_depth, invalid skipped, absent);
conventions bridge + canonicalization + fail-fast validation. Full suite green
(1504 passed).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(ingest): optional enriched schema with role:target, gated (#360)
Adds an enriched {col: {dtype, role}} shape to the schema emitted on the
global-metadata channel, behind the EMIT_ENRICHED_SCHEMA gate (default off).
The framework `label` column carries role:"target" for supervised tasks so
combine-time alignment (backend#1037) identifies the prediction target from
the contract instead of inferring it by elimination.
Breaking wire-format change (schema values: type strings -> objects), so it is
version-gated and must be cut over with backend#1037; default runs are
unchanged. Physical-table shape per the #361 discussion: keys/types are what
get_table_schema reflects, target key is `label` (distinct from feature_stats,
which keys the target by its original name).
- config.py: EMIT_ENRICHED_SCHEMA env-backed bool (default off).
- base.py: _schema_payload() builds flat or enriched; _TARGET_COLUMN="label";
role:"target" only when label_column is set (supervised).
- api/client.py: schema param typed Dict[str, Any].
- tests/test_enriched_schema.py.
Slice 1b of #360. dtype canonicalization (via #349) and the remaining
descriptors are follow-ups.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(ingest): key regression-class target feature_stats under `label` (#360)
The target is stored in the framework-standard `label` column and flagged
role:"target" in the enriched schema (PR #370), but feature_stats keyed it by
its original CSV name — so the two channels disagreed on the target key and a
consumer had to guess (tracebloc-engine's TSF scaler_y identified it by
elimination, which is ambiguous when an extra non-feature numeric stat exists).
Re-key the regression-class target's feature_stats entry to `label` at finalize
so schema.role and feature_stats agree; the engine now looks it up directly
(tracebloc-engine#460). Feature columns keep their own names.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(schema): canonical dtype in the enriched schema (#360 / #349)
The enriched schema emitted raw storage types as dtype (VARCHAR(255),
DECIMAL(10,2)), so combine-time comparison (backend#1037) would read a mere
width/size difference — VARCHAR(255) vs VARCHAR(100), or INT vs BIGINT — as a
type divergence and spuriously block.
Add schema_inference.canonical_dtype (housed with the #349 type-inference
rules): strips (...) parametrisation and folds storage-size variants to one
logical family (int/float/bool/date/datetime/time/string/binary), unknown types
passing through lower-cased. _schema_payload now emits the canonical dtype.
- schema_inference.py: _CANONICAL_DTYPE map + canonical_dtype().
- base.py: _schema_payload uses canonical_dtype.
- tests: canonical_dtype mapping + width/size folding; enriched schema emits
canonical dtypes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(schema): uploader-declared unit/ordinal descriptors in the enriched schema (#360)
unit and ordinal can't be inferred from the data, so they're declared per column
in ingest.yaml and merged into the enriched schema descriptors. This activates
backend#1037's combine-time descriptor checks — most importantly the `unit`
mismatch guard, so an income-in-USD dataset isn't silently merged with an
income-in-EUR one.
- ingest.v1.json: optional top-level `columns` map {col: {unit?, ordinal?}},
additionalProperties:false on the descriptor (typos rejected).
- conventions.py: bridge top-level `columns` onto file_options.column_descriptors
(same spec-wins precedence as the target_size / time_column bridges).
- base.py: _schema_payload merges unit/ordinal for columns present in the schema
(declared entries for unknown columns ignored); enriched branch only.
- tests: ingest.v1.json accept/reject, conventions bridge, _schema_payload merge.
Declared by CSV column name — matches feature columns; the target (keyed `label`
in the physical schema) isn't a typical unit-bearing column.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(schema): bool_encoding + null_encoding descriptors in the enriched schema (#360)
Complete the enriched schema's per-column descriptors with the two value
encodings, so backend#1037's cross-dataset WARN checks aren't inert:
- null_encoding: "null" on every column — the ingestor maps every recognized NA
token to SQL NULL.
- bool_encoding: "1/0" on bool-dtype columns — MySQL BOOLEAN stores 1/0.
Both describe the INGESTED (physical) table and are uniform by construction, so
they're derived in _schema_payload from the physical schema — no cast-pass
observation, no csv_ingestor changes. A dataset ingested under a different
convention would surface as a mismatch at combine time.
- base.py: _NULL_ENCODING / _BOOL_ENCODING constants; _schema_payload stamps
null_encoding per column and bool_encoding on bool columns.
- tests: encodings present/scoped correctly; existing descriptor assertions
updated.
Enriched schema descriptor set now complete: dtype, role, unit, ordinal,
null_encoding, bool_encoding.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ingest): address Bugbot review on #367 (categorical exclusion + bit_depth)
1. [High] Categorical vocab exclusion used exact string keys against the raw
configured label/id/annotation names, but the accumulation runs on the raw
CSV header before label pinning. A header spelled `Label` (config `label`)
slipped the exclusion and leaked the column's raw values into
attributes.feature_stats. Match via resolve_column (the #340 case-/
whitespace-insensitive rule) so the intended columns are always excluded.
2. [Low] _scalar_attribute_metadata copied any int bit_depth into emitted
attributes; a value set via spec.file_options / modality spec bypasses
conventions.resolve()'s 8/16 gate. Re-check bit_depth in (8, 16) before
emitting, mirroring color_mode canonicalisation.
- tests: label excluded from vocab under a case-mismatched header; bit_depth=12
dropped while a valid color_mode still emits.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(ingest): min-count suppression for rare categorical values (#360)
Addresses the #367 review's rare-category re-identification concern: a category
present for very few records (a rare diagnosis / demographic) was emitted
verbatim in the alignment vocab.
- The categorical accumulator now tracks per-value occurrence COUNTS (Counter,
vectorised value_counts per chunk) instead of presence-only.
- categorical_vocab() drops values seen fewer than CATEGORICAL_MIN_COUNT times;
a column left empty is omitted.
- New CATEGORICAL_MIN_COUNT config (env-backed int, default 1 = keep all / no
suppression). Default is a no-op because raising it drops rare values from the
union vocab, which the edge must then handle as OOV at encode time — that
trade needs backend privacy sign-off + engine OOV handling (backend#1079/#1053),
so it ships as an opt-in knob to flip post-sign-off.
- tests: suppression at min_count=2 (incl. across chunks), default keeps all,
column omitted when all values suppressed; config default + numeric-override
coverage.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(ingest): enriched schema on by default + emit text/survival alignment facts (#360)
G5: EMIT_ENRICHED_SCHEMA now defaults ON — backend#1037 (schema pass-through,
role-preserving) and engine#460 (reads role from schema) depend on the enriched
{col: {dtype, role}} shape. An explicit falsey env/override still forces the
legacy flat map for a pre-cutover backend.
G9/G10: emit the previously-inert alignment facts so the backend's BLOCK guards
go live and edge text handling has inputs —
- text: uploader-declared `language` + `normalization` (alongside `encoding`),
- survival: uploader-declared `time_unit` + `event_indicator` {event, censored},
bridged from config in conventions.resolve() (validated) and re-checked against
the contract shapes before emission.
Also scopes text facts to the text categories only (not embeddings, which the
contract scopes to its own positive_definition) — fixes a latent ingest-time
rejection where `encoding` was emitted for embeddings datasets.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(ingest): one canonical schema on the wire + emit embeddings positive_definition (#360)
G8: drop the internal, label-stripped `schema` copy from the shipped meta_data —
the canonical schema already ships as the top-level `schema` arg (enriched,
target-bearing). file_options keeps its copy for the in-run validators; only the
duplicate on the wire is removed, so downstream reads one unambiguous schema.
positive_definition: emit the embeddings-specific alignment fact (what defines a
positive pair, uploader-declared) for the embeddings category, bridged from
config in conventions.resolve(). Closes the last `expected` contract field that
was never produced.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ingest): resolve feature_stats exclusion + target re-key case-insensitively (#360)
Bugbot/review (#361): _accumulate_feature_stats compared the configured
label/id/annotation names against actual CSV headers with exact membership, and
the regression-class target re-key required an exact label_column match. Headers
can differ from the configured names by case/whitespace, and validators resolve
them via resolve_column — so a drifted header (config "Label" vs header "label",
or "Target"/"rowid" vs "target"/"RowId") slipped the exclusion (leaking a label/
id into feature_stats) or skipped the re-key (leaving the target under the CSV
name, breaking alignment with schema role:"target" / backend feature_stats[label]).
Both now use resolve_column, matching the categorical-exclusion path. Regression
tests cover a case-drifted classification label and a case-drifted regression
target + id.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ingest): resolve column_descriptors case-insensitively + map target descriptor to label (#360)
Bugbot (#361, commit b3cd028): the enriched-schema column_descriptors merge used
exact key membership (col not in enriched), so a descriptor keyed with case/
whitespace drift from the reflected column was dropped, and a descriptor for the
target (declared by its source name, e.g. "demand_mw") never attached because the
physical schema keys the target as `label`. Both now use resolve_column, and a
descriptor matching the configured label_column maps onto `label`.
Also fixes the CI failure: the earlier regression test drifted unique_id_column,
which the unique_id validation rejects at ingest (exact match, before
accumulation) — so a drifted id can never reach the stats to pollute them. The
test now drifts only the target (the reachable case); the id matches its header.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ingest): resolve exclusion sets once, then exact-match — no over-match (#360)
Bugbot (#361, commit e9df11f): case-insensitive resolve_column exclusion could
OVER-match — a distinct feature whose header only case-matches a role name (label
`target` vs feature `Target`) was dropped. Now the configured label/id/annotation
names are resolved to their actual header spellings once (in _validate_csv, #340),
then feature_stats/categorical exclusion is by EXACT membership against that
resolved set — so a drifted role header is still excluded while a case-variant
feature is kept. The base.py descriptor merge resolves to a physical column first,
mapping to `label` only when the declared name is the target's source name.
Regression tests: a feature case-matching the label name is kept; a feature
descriptor case-matching the label's source name is not misrouted to `label`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(ingest): flip default data_id strategy uuid → content_hash (#350)
#225 shipped the salted content-hash data_id dark in v0.5.7 — the default
data_id_strategy stayed `uuid` everywhere, so the retry-duplication cure was
inert. Privacy sign-off is approved (backend#818) and content_hash has soaked
opt-in through v0.6.0/v0.7.x, so flip the default now.
- Default data_id_strategy `uuid` → `content_hash` at all four constructor
sites (base.py, record_processor.py, csv/json_ingestor.py) and in the CLI
ResolvedConfig (cli/conventions.py).
- Add an explicit `uuid` opt-out branch to resolve(): with the default now
content_hash, `strategy: uuid` would otherwise fall through and silently
stay content_hash. `column` keeps mapping unique_id_column (which wins over
the strategy in RecordProcessor regardless of its value).
- Update schema/ingest.v1.json default + description to match.
Tests: lock the new default and the uuid opt-out in test_conventions and
test_content_hash_data_id; add a process_record content-hash default test.
Mocked-DB harnesses that exercise the full ingest() path now stub
get_or_create_table_salt (a real DB always returns a salt); process_record-
direct harnesses pre-set _table_salt or pin uuid where the id strategy is
orthogonal to what they test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ingest): don't silently clobber a feature named "label" when re-keying the regression target (#360 review)
feature_stats() re-keys a regression-class target from its CSV column name to
the reserved "label" key (so feature_stats["label"] is the target downstream).
If a DIFFERENT numeric feature was literally named "label" (allowed — create_table
excludes "label" from its reserved set), the pop→assign overwrote that feature's
aggregates silently, so combine-time normalization could use the target's stats
for the real feature. The target must still own "label" (contract), but now it
logs a warning naming the clash instead of clobbering silently. Test added.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(e2e): pin uuid on duplicate-row fixtures after the content_hash flip (#350)
The image_classification (576 rows = 6 unique ×96) and object_detection
(128 = 10 unique) template fixtures repeat the same rows. Under the new
content_hash default those collapse to the distinct set via the data_id
UNIQUE upsert (content-level dedup, working as designed), breaking the
row-per-source-row goldens in the characterization + content-compare
harnesses.
Pin those two cases to data_id.strategy=uuid so the 1:1 fidelity goldens
hold. Every unique-row case keeps the content_hash default, so the full
run.main() stack is still exercised with content_hash end to end; the
content_hash dedup/retry-reclaim path is covered by
e2e/test_database_e2e.py::test_content_hash_retry_reclaims_rows.
Verified: full e2e suite (49 tests) green against real MySQL.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ingest): strip label/annotation/id columns by resolve_column so a target descriptor can't stick to the source column (#360 review)
__init__ removed the label/annotation/unique_id columns from the physical table
schema by EXACT key. label_column isn't pinned to the real header until
_resolve_label_column runs mid-ingest, so a manifest label_column that drifts in
case/whitespace from the header (e.g. "Price" vs "price") missed here and left
the target's SOURCE column in the table. It then reflected back into the enriched
schema, and _schema_payload attached the uploader's unit/ordinal descriptor to
that feature-named column instead of the framework `label` column that carries
role:"target" — so the backend's combine-time target descriptor checks silently
missed the declared unit/ordinal.
Resolve each special column against the schema keys case-/whitespace-insensitively
(the #340 rule used everywhere else) before removing it. The target source column
is then never physical, so _schema_payload's existing physical-first routing sends
the target descriptor to `label` with no #340 regression for genuine features.
Test added.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(schema): declare the top-level alignment facts so ingest.yaml can set them (#360 review)
conventions.resolve() bridges color_mode, bit_depth, language, normalization,
time_unit, event_indicator and positive_definition from the TOP LEVEL of the
ingest config into file_options, but ingest.v1.json is additionalProperties:false
and declared none of them (only `columns`). A submission using those documented
fields failed schema validation before resolve() ran, so the alignment facts
couldn't be set via the top-level surface at all.
Declare the seven keys with constraints matching resolve()'s own validation
(bit_depth enum {8,16}; time_unit enum; event_indicator object requiring
event+censored; color_mode a permissive string since canonical_color_mode accepts
case-insensitive aliases; language/normalization/positive_definition free strings).
Tests: the facts validate on their category examples, and bad values (bit_depth 12,
event_indicator missing censored) are still rejected at config time.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(objdet): resolve annotation by stem so extension-bearing filename works (#379)
object_detection ingestion skipped every record (Job exit 9, 0 ingested)
when the manifest `filename` column carried the image extension (e.g.
`a.jpg`) — the format the objdet-ok parity fixture ships and the template
README documents as supported.
`_find_src("annotations", filename, ".xml")` kept the recognised `.jpg`
extension instead of swapping to `.xml`, so it searched `annotations/a.jpg`
(never present; the annotation is `a.xml`). The File Pairing validator pairs
image<->XML by on-disk stem, so validation passed while transfer failed.
Add a `force_extension` mode to `_find_src` that strips a recognised trailing
extension and appends the target extension, and use it at both annotation
resolution sites (object_detection factory + annotation_transfer). Both the
extension-bearing `a.jpg` and the bare stem `a` now resolve to `a.xml`,
matching Path.stem (what File Pairing pairs on) and the documented
"with or without extension" contract.
Regression tests cover the end-to-end objdet extension-bearing case, the
standalone annotation_transfer path, and the _find_src stem/bare/internal-dot
edges.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ingest): reclaim staged source after verified load (~2x PVC)
The ingestor COPIES each file-bearing dataset's staged sidecars from
SRC_PATH into the final table dir (DEST_PATH) but never removed the
staging copy, so every ingest left two copies on the shared PVC and
~doubled disk usage for image / detection / segmentation datasets (#346).
Add file_transfer.reclaim_source(cfg): after a VERIFIED, clean load it
rmtree's the SRC_PATH staging tree, replacing the CLI's extra rm -rf pod
(tracebloc/cli#167) and closing the leak for the helm-driven path too.
BaseIngestor._ingest_with_lock calls it only when dataset_registered is
True AND there are no failed records — a partial/failed run keeps its
source for retry/inspection, matching the compensating-delete branch that
leaves staged files in place. The reclaim runs while the table lock is
still held, so it can't race a concurrent ingest of the same table.
reclaim_source is heavily guarded + best-effort: it never deletes a dir
that IS or CONTAINS the freshly-written table (SRC == DEST, DEST inside
SRC, SRC == STORAGE_PATH), never touches the shared root, and swallows
filesystem errors so a leftover staging copy can't turn a green ingest
red. No per-file shutil.move (that would delete source mid-load, before
the load is verified — unsafe).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ingest): make source reclaim opt-in to the staging subtree (Bugbot #381)
reclaim_source used a blocklist of dangerous layouts; Bugbot found three
holes where a clean load would rmtree the wrong tree:
- the helm layout puts SRC_PATH at the user's OWN dataset dir (parent of
images:, e.g. /data/shared/cats-dogs), a SIBLING of the table dir — the
blocklist only skipped when DEST was INSIDE SRC, so it deleted user data;
- guards compared os.path.abspath strings, so a symlinked SRC_PATH (PVC
mounts are symlinks) resolved past them and rmtree could hit the real target;
- dest_abs.startswith(src_abs + os.sep) degenerates to a "//" prefix when
SRC_PATH is "/", so a root SRC slipped the containment check.
Replace the blocklist with a positive, opt-in gate: reclaim ONLY a dir that
resolves (realpath) to strictly inside STORAGE_PATH/.tracebloc-staging — the
isolated SharedRoot/.tracebloc-staging/<table> tree the CLI stage pod writes,
provably a throwaway copy. All paths are realpath'd before every check and
before rmtree (fixes the symlink bypass); containment uses commonpath
component-wise (fixes the "//" root case); a backstop still refuses if the
resolved source overlaps the table dir.
Net: the CLI-staged path (the #346 target, replacing cli#167's rm -rf pod)
still reclaims; a user's mounted data dir in the helm-direct layout is left
untouched — no worse than the pre-#346 status quo.
Adds tests for the sibling user dir, the symlink escape, a root SRC, and a
symlinked storage mount (must still reclaim).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ingest): bind source reclaim to the per-table staging dir (Bugbot #381)
The opt-in gate proved SRC_PATH was under STORAGE_PATH/.tracebloc-staging but
not that it was THIS table's staging dir, so a stray SRC_PATH pointing at
another dataset's staging — or a <table> symlink that realpaths into the tree —
could pass and be rmtree'd on the shared PVC.
Require the resolved SRC_PATH to equal EXACTLY
STORAGE_PATH/.tracebloc-staging/<TABLE_NAME> (the literal per-table path, NOT
realpath'd, so a <table> symlink pointing elsewhere fails the equality and is
left alone). A blank TABLE_NAME, another table's dir, or a name that escapes the
staging subtree all skip. The dest-overlap backstop and best-effort rmtree are
unchanged.
Adds tests: another table's staging dir, a symlink into another table's staging,
and an unset TABLE_NAME.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ingest): make the WHOLE reclaim best-effort, not just rmtree (Bugbot #381)
reclaim_source promised best-effort ("a leftover staging copy must never turn a
green ingest red") but only shutil.rmtree was guarded. realpath, isdir, the
_is_within guards, or a logging call could still raise and propagate through the
ungated call in _ingest_with_lock AFTER the dataset is registered — failing a
load that already succeeded.
Split the guard/decision/delete logic into _reclaim_source and wrap the whole
call in reclaim_source with a catch-all that logs (itself guarded, so a broken
logger can't propagate either) and returns False.
Adds tests: an unexpected guard error and a logging failure are both swallowed
without failing the ingest.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ingest): resolve filename column case-/whitespace-insensitively end-to-end (#372)
The filename column was validated case-/whitespace-insensitively at
preflight (IngestableRecordsValidator via the shared resolve_column rule)
but read by the exact literal key record.get("filename") at transfer, so
a header like `Filename` passed the dry-run and then failed at transfer
for every row ("No filename found in record", exit 9) after upload — the
#340 class already fixed for the label column, still open for filename.
Add BaseIngestor._resolve_filename_key, wired into the ingest loop next
to the #340 label resolver: it resolves the file-pointer column once per
run via the same resolve_column rule the validators use and copies it
onto the canonical `filename` key every downstream read expects, so
preflight and transfer agree. Scoped to file-bearing categories; no-op
for the common lowercase header, for an image_id-only manifest (a
genuinely different column, not a case variant — cli#371), and for
tabular datasets carrying an unrelated FileName data column. Sparse JSON
whose leading object omits the column is retried on later records.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(objdet): soften Path.stem claim in _find_src force_extension comment
Comment-only. `_has_extension` strips only a recognised trailing extension, so
the stem matches Path.stem for the jpg/jpeg/png images objdet ships but not for
a non-recognised final suffix (e.g. a.tar.gz) — unreachable for objdet, but the
"matches Path.stem exactly" wording shouldn't be leaned on. Reword to state the
recognised-extension gate, why it's deliberately NOT Path.stem (keeps
image.001 whole), and where the two diverge. Addresses Asad's review nit on #380.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ingest): warn loudly when a staged-but-unbindable source isn't reclaimed (#381)
Addresses Asad's review: the per-table gate silently couples reclaim to the
CLI's staging layout (SRC_PATH == SharedRoot/.tracebloc-staging/<TABLE_NAME>),
and nothing here pins that cross-repo contract. If the CLI convention drifts,
reclaim quietly no-ops at info level and the #346 ~2x-disk leak returns on a
green ingest.
- Split the skip log: a source UNDER .tracebloc-staging but not this table's
expected dir (a CLI layout drift, or a cross-table SRC_PATH) now logs WARNING,
so the #346 leak can't return invisibly on a green ingest. A source OUTSIDE
the tree (helm user dir, storage root) stays INFO (a deliberate skip).
- Documented the cross-repo coupling at STAGING_DIRNAME and flagged a follow-up
for a shared constant / cross-repo integration check.
Adds tests: drift under the staging tree warns; a user-dir skip stays quiet.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(csv): gate alignment feature_stats/vocab to tabular-family categories
feature_stats and the categorical union vocab are combine-time alignment
facts (backend#1037) that describe the CSV's cells as FEATURES — only true
when the rows are the data (ModalitySpec is_tabular_family). They were
accumulated and emitted for EVERY CSV category, so a manifest-style ingest
(e.g. keypoint_detection with a Visibility JSON TEXT column) shipped raw
cell content off-premise as a "vocab" for no alignment gain.
Gate both accumulators on TABULAR_FAMILY_CATEGORIES (the registry's
single-source set base.py already uses). Bugbot medium on the #383 release
PR.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(ingest): stop shipping the internal column_descriptors bridge in meta_data
column_descriptors is bridged onto file_options by config resolution solely
so _schema_payload can merge the uploader's unit/ordinal declarations onto
the enriched schema's columns — but the send path stripped only the internal
"schema" copy, so the raw descriptor map (keyed by CSV source names the
backend can't correlate) still shipped in meta_data after its facts were
already on the schema.
Extract the filter into _meta_data_payload() with a shared
_META_DATA_INTERNAL_KEYS set covering both bridges, unit-testable without a
full ingest run. Bugbot medium on the #383 release PR.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Merge pull request #386 from tracebloc/feat/persist-ingest-task
feat(journal): record each run's task in the ingest-run journal
* fix(ingest): strip the scalar alignment bridges from meta_data too
Second bugbot round on #383, same internal-bridge class #385 closed for
schema/column_descriptors: color_mode, bit_depth, language, normalization,
time_unit, event_indicator and positive_definition are bridged onto
file_options only so _scalar_attribute_metadata can copy them under
attributes — the sole channel the backend reads (dataset_validators reads
meta_data.attributes). The raw keys still shipped at meta_data top level
beside the canonical copies. Extend _META_DATA_INTERNAL_KEYS to cover all
bridged facts.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Merge pull request #384 from tracebloc/fix/372-filename-column-exact-lowercase
fix(validators): require an exact lowercase "filename" column at preflight (#372, supersedes #382)
* fix(objdet): default data_id to uuid — content_hash collapses per-object rows
The #350 flip made content_hash the default for every category, but objdet
manifests list one row PER OBJECT: duplicate (filename, label) rows are
distinct objects (the bundled VisDrone sample has three identical car rows
for one image). Those rows produce the same content digest, so the data_id
UNIQUE upsert keeps a single stored row — silently under-counting objects
and label_counts. Bugbot High on the #383 release PR; the e2e fixtures
already had to pin uuid to dodge exactly this.
- conventions.resolve: absent data_id block + object_detection => uuid
(explicit strategy: content_hash stays honored)
- objdet template: pass data_id_strategy="uuid" explicitly
- BaseIngestor: warn loudly when objdet runs under content_hash via the
direct-constructor path (can't distinguish explicit from default there)
Restoring retry idempotency for objdet (row-ordinal-salted hash) is a
follow-up, not a default.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(objdet): import TaskCategory locally — merge-ref loses the module import
develop's copy of test_ingestor_base.py was refactored to function-local
TaskCategory imports and has no module-level one, so the PR merge-ref
resolves the import block to develop's version and the new test
NameErrors (pytest 3.11/3.12 on the PR). Make the test self-contained,
matching the file's convention.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(backfill): metadata builder for pre-cutover datasets (di#360 / backend#1037) (#378)
Adds tracebloc_ingestor/metadata_backfill.build_dataset_metadata: recompute the
{schema, meta_data} payload for an ALREADY-INGESTED table without re-ingesting
rows. Numeric feature_stats come from SQL aggregates, categorical vocab from a
bounded GROUP BY. It reuses CSVIngestor._schema_payload / _collect_run_metadata
(by injecting SQL-computed accumulators) so the output is byte-identical to a
fresh ingest. The API send is intentionally NOT included.
Review refinements (#378):
- Square in float (DOUBLE) via cast so sum_sq matches the live float64 path and
can't overflow BIGINT / drift on INT columns.
- LIMIT the categorical GROUP BY to cap+1 so a high-cardinality column isn't
fully loaded just to be dropped.
- Import _MAX_CATEGORICAL_CARDINALITY from CSVIngestor (no duplicate constant).
- Note on CSVIngestor's _feature_stats_acc/_categorical_acc/_schema_payload that
metadata_backfill depends on them (rename protection).
Tests: unit-cover the SQL-shaping helpers (framework/label exclusion, regression
target kept as "label", int/float + Decimal coercion, all-null omission,
cardinality cap) with a fake connection, plus SQLite integration tests
end-to-end (regression + classification), asserting exact sum_sq.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix(backfill): honor the tabular-family alignment gate (#385) in metadata backfill
build_dataset_metadata injected the SQL-built accumulators directly into
the ingestor, bypassing the accumulation-time gate #385 added on the live
cast pass — so a backfill pointed at a manifest-style table (e.g. keypoint
with a Visibility TEXT column) could ship raw cell values as vocab that a
fresh ingest of the same table would suppress. Bugbot High on the #383
release PR.
Honor the ingestor's own _emit_alignment_stats flag (derived from the
category in __init__) and skip the table scans entirely for non-tabular
categories, so the backfilled payload mirrors a fresh ingest's.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* chore(release): bump version to 0.7.5 (#390)
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Divya <divyasingh@tracebloc.io>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: lukasWuttke <54042461+LukasWodka@users.noreply.github.com>
Co-authored-by: Lukas Wuttke <lukas@tracebloc.io>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@saadqbalsaadqbalsaadqbal approved these changes

Assignees

@LukasWodkaLukasWodka

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@LukasWodka@saadqbal