feat(backfill): metadata backfill runner — GET dataset → recompute → send to backend (#378 send side)#393
Merged
Conversation
Collaborator
|
👋 Heads-up — Code review queue is at 32 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
…nd 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>
divyasinghds
force-pushed
the
feat/metadata-backfill-runner
branch
from
July 23, 2026 11:26
e644977 to
64e171b
Compare
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>
divyasinghds
marked this pull request as ready for review
July 23, 2026 12:29
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b5d88e8. Configure here.
… 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>
saadqbal
approved these changes
Jul 23, 2026
This was referenced Jul 24, 2026
Collaborator
Functional review \u2014 passed\n\nBasis: the behavioural suites that ran on this PR at merge against real environments, not mocks:\n\n- \u2014 exercised against a real MySQL 8.0 service container, not a mock\n\nI could not reach the dev API (no dev credentials), so rather than rubber-stamp I used the strongest evidence available: these suites exercise the actual behaviour this change alters, on real infrastructure. Advancing \u2192 .\n\nIf the functional reviewer wants a manual pass on dev in addition, please move it back and say so. |
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 & why
Datasets ingested before the federated-alignment cutover carry no enriched schema and no per-column
feature_statson the global-metadata channel, so the backend combine-time alignment checks and the edge metadata API can't reconcile them. #378 (build_dataset_metadata) recomputes that{schema, meta_data}for a table from its persisted rows — but deliberately stopped before sending. This PR is the runner that ties it to the backend.For each dataset it does exactly what was asked — get data from backend → based on category compute all attributes → send updated attributes back:
ingestor_id(GET global_meta/metadata_backfill/by-ingestor/<id>/, #1198). The authoritativecategory+data_formatdrive the recompute and aren't recoverable from the table alone.{schema, meta_data}from persisted rows viabuild_dataset_metadata(SQL aggregates, no row re-ingest), keyed on that category.POST global_meta/metadata_backfill/<table>/, #1166) — the endpoint upsertsGlobalMetaDataand re-folds any competition built from the table (#1198).What this PR adds
APIClient.get_dataset_metadata(ingestor_id)Noneon 404 (skip, not error); local-mode mockAPIClient.send_metadata_backfill(table, schema, meta_data){table_name, created, competitions_refolded}Database.list_registered_runs()metadata_backfill_runner.backfill_datasetmetadata_backfill_runner.backfill_datasetstracebloc-backfillconsole scriptingest.yamlSkips & guards
is_competition/source_dataset_ids): they have no client-side raw table to read, and their reconciled fold is rebuilt backend-side by the re-fold the source POSTs trigger.meta_data.attributespresent) so a re-run is cheap; disable withskip_if_current=False.Scope / non-goals
label_columnand the uploader-declared scalar attributes (color_mode,language, …) aren't persisted backend-side, so they aren't passed; the SQL-derivable enriched schema +feature_stats(the backfill's primary value) are recomputed regardless. Same per-fact limitations asbuild_dataset_metadata's docstring.What reviewers should focus on
list_registered_runs()(registered ingest journal) as the set to sweep. Right boundary, or should the operator pass an explicitingestor_idlist?is_competition or source_dataset_idsthe right detector for "no local table to recompute"?skip_if_currentmarker — using non-emptymeta_data.attributesas "already new-shape." Conservative (a category with legitimately-empty attributes re-POSTs, idempotently). Acceptable?tracebloc-backfillconsole script (vs. a subcommand ontracebloc-ingest), reading creds from env.Tests
test_metadata_backfill_runner.py— happy path (GET→build→POST with category/data_format threaded), not-found, competition/merged skip, already-current skip (+skip_if_current=Falsereprocess), missing-category error, journal enumeration, and per-dataset guard (a failing dataset doesn't abort the sweep).test_api_client_methods.py— the two new methods: success / 404→None / HTTP-error / local-mode / payload shape.test_database.py—list_registered_runsreturns registered-only, mapped, NULL task preserved.Full suite green (1866 passed).
Rollout / pairing
develop).tracebloc-backfillin the ingestor image's env. Non-zero exit iff any dataset errored.🤖 Generated with Claude Code
Note
Medium Risk
Rollout job mutates global metadata on the backend for every registered dataset; failures are isolated but a bad recompute or POST could affect alignment/competition folds until re-run (idempotent).
Overview
Adds a one-time rollout sweep for pre-cutover datasets: for each registered ingest, GET backend metadata by
ingestor_id, recompute{schema, meta_data}viabuild_dataset_metadatafrom local MySQL, then POST to the metadata-backfill API (upsert + competition re-fold).New pieces:
APIClient.get_dataset_metadata/send_metadata_backfill(404 → skip; HTTP errors omit response bodies in messages for log safety),Database.list_registered_runs(registered = 1journal rows),metadata_backfill_runnerwith skip rules (competition/merged, already hasmeta_data.attributes), per-dataset error isolation, andtracebloc-backfillentrypoint using the same env as the ingestor (noingest.yaml).Reviewed by Cursor Bugbot for commit e1aa20e. Bugbot is set up for automated code reviews on this repo. Configure here.