Skip to content

fix: Bugbot findings from promotion PR #396 - #397

Merged
shujaatTracebloc merged 1 commit into
developfrom
fix/bugbot-validator-labels
Jul 24, 2026
Merged

fix: Bugbot findings from promotion PR #396#397
shujaatTracebloc merged 1 commit into
developfrom
fix/bugbot-validator-labels

Conversation

@shujaatTracebloc

@shujaatTracebloc shujaatTracebloc commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Resolves the Cursor Bugbot finding surfaced on the data-ingestors promotion PR (#396, develop → master).

Fixes:

  • Validator log labels drop suffix — normalize the label at the log site (append "Validator" only when not already present), so validator names that don't end in "Validator" keep the suffix. (Release develop → master #396 (comment))

Lands on develop; the promotion PR head picks it up on the next sync and Bugbot re-reviews.

🤖 Generated with Claude Code


Note

Low Risk
Logging and user-facing validation messages only; no change to validation logic or ingestion behavior.

Overview
Validator preflight messaging now uses a shared _validator_label() helper so every run/fail/warning line shows a consistent human-readable name.

Validator .name values were mixed: some already end in "Validator" (e.g. Data Validator) and many do not (e.g. Ingestable Records, Text Content). The previous success path assumed names were already suffixed and stopped appending "Validator", which dropped the word for unsuffixed names while still risking doubled text for suffixed ones. validate_data now normalizes once per validator and uses that label for running, failure, warning, success, and exception messages—appending "Validator" only when missing (case-insensitive), with whitespace stripped first.

Tests cover _validator_label cases and integration checks on success/failure output.

Reviewed by Cursor Bugbot for commit 90c3ecf. Bugbot is set up for automated code reviews on this repo. Configure here.

Append "Validator" only when validator.name doesn't already end in it,
so names like "Ingestable Records" keep the suffix and names like
"Data Validator" don't double it.

- tracebloc_ingestor/ingestors/base.py

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@shujaatTracebloc
shujaatTracebloc merged commit 0a6bed2 into develop Jul 24, 2026
10 checks passed
@shujaatTracebloc
shujaatTracebloc deleted the fix/bugbot-validator-labels branch July 24, 2026 13:13
shujaatTracebloc added a commit that referenced this pull request Jul 27, 2026
* fix(validators): stop doubling "Validator" in validation log lines (D1) (#392)

validator.name already ends in "Validator" (e.g. "Data Validator"), so appending
the word again produced "Data Validator Validator successfully passed" (and the
same doubling in the failed/error paths). Drop the redundant literal in all
three spots so the lines read "Data Validator successfully passed" /
"Data Validator failed: …" / "Data Validator error: …". Log-string only, no
logic change; updated the one docstring that quoted the old doubled message.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* feat(backfill): metadata backfill runner — GET dataset → recompute → send to backend (#378 send side) (#393)

* feat(backfill): metadata backfill runner — GET dataset, recompute, send to backend

Wires data-ingestors #378 (build_dataset_metadata) to the backend metadata
endpoints (#1166 POST, #1198 GET) into a one-time, per-client pre-cutover
backfill sweep. For each registered dataset it:

  1. GETs the backend record by ingestor_id (authoritative category +
     data_format, not recoverable from the table alone);
  2. recomputes {schema, meta_data} from persisted rows via
     build_dataset_metadata, keyed on that category;
  3. POSTs it back — the endpoint upserts GlobalMetaData and re-folds any
     competition built from the table.

- APIClient.get_dataset_metadata / send_metadata_backfill — the two API calls,
  with local-mode mocks, 404→None (skip), and error surfacing.
- Database.list_registered_runs — enumerate the client's REGISTERED ingest
  journal (the datasets to sweep).
- metadata_backfill_runner — backfill_dataset (GET→build→POST, skips
  competitions/merged and already-new-shape datasets) + backfill_datasets
  (per-dataset guarded sweep) + a `tracebloc-backfill` console-script entrypoint
  reading DB/backend creds from env (no ingest.yaml).

Idempotent and re-runnable; new ingests already emit new-shape metadata, so this
is a rollout step, not always-on.

Tests: runner orchestration/skip/guard paths, the two API methods
(success/404/error/local/payload shape), and the DB enumeration.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore: retrigger checks after retargeting PR base to develop

The FR gate (staging/prod-promotion guard) ran against the original master
base and its stale failure lingered on the head commit after retargeting to
develop, where the gate does not apply. Empty commit to reset the head SHA.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(backfill): don't leak raw exception text / response bodies in the sweep (Bugbot #393)

Per the org rule "error messages and logs must never embed customer cell values":

- Runner: on a per-dataset failure, record and log the exception TYPE only
  (`type(exc).__name__`) instead of `str(exc)[:500]`, and use logger.error
  rather than logger.exception (no traceback). This runs in a Helm-hook Job
  whose output lands in install logs.
- APIClient.get_dataset_metadata / send_metadata_backfill: raise
  HTTPError("HTTP <status>") without response.text. The backfill POST body
  carries this table's categorical vocab (customer cell values), so a backend
  error echoing it back must not reach the exception string / logs. The response
  object is still attached for deliberate programmatic inspection.

Test asserts the recorded error is the exception type ("RuntimeError"), proving
the raw message ("boom") is not surfaced.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* chore(release): bump version to 0.7.6 (#394)

* chore(release): bump version to 0.7.6

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(image-validator): auto-detect from first readable image, not files[0]

test_too_small_result_still_reports_corrupt_files was order-dependent:
auto-detection read only image_files[0], so when a corrupt file came
first in glob order (deterministic on the Linux CI, not on macOS) it
returned None, expected_resolution stayed unset, and
_validate_image_resolutions bailed with "auto-detection failed" before
the min-size floor check could run — masking the real verdict and
making the outcome depend on filesystem ordering.

Scan for the first *readable* image instead; if none are readable,
expected_resolution stays unset and the honest "auto-detection failed"
still surfaces. Adds an order-independence guard test that forces the
corrupt file first (fails pre-fix, passes post-fix).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: shujaat <shujaat@tracebloc.io>

* feat(backfill): discover dataset tables directly, not just the runs journal (#395)

* feat(backfill): discover dataset tables directly, not just the runs journal

The runner enumerated the runs journal (list_registered_runs), which only has
entries for datasets ingested by the post-journal ingestor. The pre-cutover
backlog the backfill exists for was ingested BEFORE the journal, so it had no
entry and was silently swept 0 (verified on dev: empty journal, ~170 real
dataset tables untouched).

Enumerate dataset tables directly instead:
- Database.list_dataset_ingestor_ids() — scan information_schema for tables
  carrying the framework `ingestor_id` column (excluding the run journal + salt
  store), then read each table's distinct non-null ingestor_ids. Covers
  journalled AND pre-journal datasets (the journal is a strict subset).
- backfill_datasets() defaults to this discovery. A table appended by several
  ingest runs has multiple ingestor_ids; metadata is table-level, so a table is
  backfilled once — the first id the backend resolves wins, siblings are skipped
  (a not_found/error id doesn't mark the table done, so another id can resolve
  it). Explicit ingestor_ids still supported.

Tests: table discovery is the default; framework tables excluded; multi-id table
backfilled once; sibling id retried after a not_found.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(release): bump version to 0.7.7

Bundled with the table-discovery change so a single merge + v0.7.7 tag yields a
deployable image carrying it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(backfill): guard per-table discovery so one bad table can't abort the sweep (Bugbot #395)

list_dataset_ingestor_ids scanned every dataset table with no per-table guard, so
a single failing SELECT DISTINCT (scan timeout, table dropped mid-sweep, perms)
aborted enumeration and skipped ALL tables. Wrap the per-table query in
try/except: log the table name + exception TYPE only (never str(exc) — a driver
message can embed cell values and feeds install logs) and continue.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* fix: normalize validator log labels (Bugbot, promotion PR #396) (#397)

Append "Validator" only when validator.name doesn't already end in it,
so names like "Ingestable Records" keep the suffix and names like
"Data Validator" don't double it.

- tracebloc_ingestor/ingestors/base.py

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: lukasWuttke <54042461+LukasWodka@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Divya <divyasingh@tracebloc.io>
shujaatTracebloc added a commit that referenced this pull request Jul 27, 2026
Ships the validator log-label normalization fix (#397) — the only code on
develop/master ahead of the v0.7.7 release image. Tagging v0.7.8 after this
merges triggers release-image.yml -> ghcr.io/tracebloc/ingestor:0.7.8.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants