Skip to content

Phase 1: embed ingest.v1.json + tracebloc ingest validate - #1

Merged
saadqbal merged 4 commits into
developfrom
feat/149-schema-validator
May 21, 2026
Merged

Phase 1: embed ingest.v1.json + tracebloc ingest validate#1
saadqbal merged 4 commits into
developfrom
feat/149-schema-validator

Conversation

@saadqbal

@saadqbalsaadqbal commented May 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

Phase 1 of the v0.1 roadmap (tracebloc/client#147). Brings the cluster's jobs-manager schema validation onto the customer's workstation: a customer can run tracebloc ingest validate ./my-ingest.yaml and get the same error messages jobs-manager would emit, but locally and in milliseconds.

What lands

  • scripts/sync-schema.sh — pulls ingest.v1.json from tracebloc/data-ingestors master into internal/schema/ingest.v1.json. --check mode exits non-zero on drift; wired into CI as a new schema-drift job so silent divergence between the CLI's view and the canonical schema becomes a PR-blocking signal.
  • internal/schema/ — embedded schema bytes + a Validator that compiles once and validates many. Walks the jsonschema/v6 error tree to produce a flat []ValidationError{Path, Message} whose output format matches tracebloc_ingestor.cli.run._format_errors byte-for-byte (so customer-facing wording stays uniform across the two validator implementations).
  • internal/cli/ingest.gotracebloc ingest validate <path> subcommand. Three exit codes: 0 ok, 2 schema violations (printed to stderr), 3 file/parse problems.
  • internal/cli/exit.goexitError type + ExitCodeFromError helper for subcommand → main() exit-code propagation. Wired through cmd/tracebloc/main.go.

Test plan

  • 23 new test cases across 2 files, all green locally. Coverage: internal/schema 80%, internal/cli 88%.
  • Happy path across 6 categories (image, OD, semantic seg, text, tabular classification, tabular regression-with-bucket-policy)
  • 5 negative-case schema rules (missing required, enum mismatch, image-category-without-images, regression-without-policy, tabular-without-schema)
  • 4 parse-failure modes (empty, whitespace, top-level sequence, malformed YAML)
  • Format contract pin (FormatErrors output shape)
  • CLI dispatch + exit-code tests
  • scripts/sync-schema.sh --check passes against current master of data-ingestors

Library choices

  • github.com/santhosh-tekuri/jsonschema/v6 — the standard Go JSON Schema impl, supports draft-07 (which ingest.v1.json targets). One footgun: ErrorKind.LocalizedString(nil) segfaults inside golang.org/x/text (likely a library bug). Worked around defensively with a package-level English *message.Printer.
  • golang.org/x/text pinned to v0.16.0 — Go 1.22-compatible. The latest text v0.37 requires Go 1.25 which would force the module's minimum version up.

Customer-facing UX (preview)

$ tracebloc ingest validate ./bad.yaml
./bad.yaml: schema validation failed (1 issue)
<root>: missing properties 'apiVersion', 'intent'
$ echo$?
2
$ tracebloc ingest validate ./good.yaml
./good.yaml: ok
$ echo$?
0

Closes

tracebloc/client#149

Next phase

Phase 2 (tracebloc/client#150) — cluster discovery via kubeconfig + ingestor SA token via TokenRequest. That unlocks the actual tracebloc dataset push flow (Phases 3 + 4).

🤖 Generated with Claude Code


Note

Medium Risk
Adds a new user-facing validation command with custom exit-code/error-printing behavior and introduces JSON Schema/YAML parsing dependencies, which could affect CLI UX and error handling edge cases. Also adds a CI drift gate that may block PRs if the upstream schema changes unexpectedly.

Overview
Adds offline ingestion config validation via tracebloc ingest validate, backed by an embedded ingest.v1.json JSON Schema and a validator that formats violations consistently and returns distinct exit codes for schema vs file/parse errors.

Updates process-level error handling so cobra-silenced errors are still surfaced (while allowing handlers to opt into silent errors) and propagates command-specific exit codes from main().

Introduces scripts/sync-schema.sh plus a new CI schema-drift job and Makefile targets to keep the embedded schema in sync with tracebloc/data-ingestors, alongside updated contributor docs and new Go module dependencies (jsonschema/v6, yaml.v3, x/text).

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

Phase 1 of the v0.1 roadmap. Brings the cluster's jobs-manager
schema validation onto the customer's workstation: instant local
feedback on ingest.yaml errors, no cluster round trip needed.
What lands:
- scripts/sync-schema.sh — pulls ingest.v1.json from
tracebloc/data-ingestors' master and writes it to
internal/schema/ingest.v1.json. Has a --check mode that exits
non-zero on drift, wired into CI as the new "schema-drift" job
so silent divergence between the CLI's view and the canonical
schema becomes a PR-blocking signal.
- internal/schema/embed.go — go:embed the JSON bytes; future v2
support hangs off here without changing the package's public API.
- internal/schema/validate.go — Validator wrapping a compiled
jsonschema.Schema, walking the library's error tree to produce a
flat list of ValidationError{Path, Message} that matches the
format tracebloc_ingestor.cli.run._format_errors emits in Python
(so customer-facing wording stays uniform across the two
validators).
- internal/cli/ingest.go — `tracebloc ingest validate <path>`
subcommand with three exit codes: 0 ok, 2 schema violations
(printed to stderr), 3 file/parse problems (so callers can branch
on the cause).
- internal/cli/exit.go — exitError type + ExitCodeFromError helper
for subcommand → main() exit-code propagation.
Tests:
- internal/schema/validate_test.go: happy path across 6 categories,
5 negative-case schema rules, 4 parse-failure modes, format
contract pin, plus a local-only test against the real example
YAMLs in tracebloc/data-ingestors if checked out alongside.
- internal/cli/ingest_test.go: cobra dispatch, exit-code mapping,
arg-count enforcement.
Local coverage: internal/schema 80%, internal/cli 88%.
Library choices:
- github.com/santhosh-tekuri/jsonschema/v6 — the standard Go JSON
Schema impl, supports draft-07 (which is what ingest.v1.json
targets). Requires a *message.Printer for ErrorKind.LocalizedString;
we use a package-level English printer rather than passing nil
(which segfaults inside golang.org/x/text — likely a library bug,
worked around defensively).
- golang.org/x/text pinned to v0.16.0 (Go 1.22-compatible). The
latest text v0.37 requires Go 1.25.
Closestracebloc/client#149.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
Contributor

👋 Heads-up — Code review queue is at 18 / 8

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.)

Comment threadinternal/schema/validate_test.go Outdated
Comment threadinternal/cli/ingest.go
saadqbaland others added 2 commits May 21, 2026 18:13
Three more unchecked fmt.Fprintf/Fprintln calls in ingest.go that
golangci-lint flagged after the schema-validator PR opened. Discard
the errors explicitly (`_, _ = fmt.Fprintf(...)`) with a comment
explaining why we don't propagate them — the exit code is the
contract; a downstream pipe-write failure shouldn't invert success
into failure for reasons unrelated to validation.
gofmt -s also wanted the numbered list in internal/schema/embed.go's
package comment re-indented (3 spaces → 2 spaces after the list
marker, which is what the simplifier prefers for Go doc-comment
ordered lists).
Found by CI on PR #1, same class of bug as the Phase 0 lint-fix
commit; once tests + lint stay green this should land.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sync
Twice in the last two PRs (#0 bootstrap, #1 schema validator) lint
errors only surfaced after the push because there was no local
command that ran the exact same checks CI runs. The fix is making
`make ci` the single trusted local pre-flight: same checks, same
ordering, same exit codes.
Targets:
ci vet + test + lint + fmt-check + schema-check
(composite; what CONTRIBUTING tells devs to run pre-push)
build produces ./tracebloc
install installs via `go install` for $GOPATH/bin
vet | test isolation runs of the underlying checks
lint golangci-lint run; friendly install hint if missing
fmt gofmt -s -w . (fixes drift in place)
fmt-check gofmt -s -l . | grep -q . && exit 1 (read-only check
that mirrors CI's gofmt enforcement)
schema-check / schema-sync wraps scripts/sync-schema.sh
clean removes tracebloc, dist/, coverage files
The lint target checks `command -v golangci-lint` first and prints
an install hint on failure — caught my own setup not having it.
CONTRIBUTING.md updated: the local-dev section now leads with
`make ci`, makes the dependency on golangci-lint explicit, and
notes that skipping pre-push has cost us at least one fix-up commit
per bug class so far. Bare `go build / go test` examples replaced
with their `make`-target equivalents.
No source-code changes; purely process.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 42d9f75. Configure here.

Comment threadinternal/schema/validate.go
@saadqbalsaadqbal self-assigned this May 21, 2026
Three findings, all valid:
1. HIGH — silent error swallowing (cmd/tracebloc/main.go, internal/cli/ingest.go,
internal/cli/exit.go)
SilenceErrors: true on the root command, combined with main()
calling os.Exit(code) without printing the error first, meant
every non-schema-violation failure (file-read errors, YAML parse
errors, schema-compile bugs) produced a bare non-zero exit with
NO message to the customer. Customer thinks "tracebloc ingest
validate ./missing.yaml" silently failed.
Fix: add cli.IsSilentError(err) helper that returns true only
when an exitError carries a nil inner err (the "I already
printed my structured diagnostic" signal). main() prints
"Error: <message>" to stderr for everything else before
os.Exit'ing.
Verified end-to-end: nonexistent file now produces
"Error: reading ...: no such file or directory" + exit 3;
schema-violation path unchanged (per-violation lines + exit 2,
no duplicate "Error:" line).
2. MEDIUM — hardcoded developer-local path in test
(internal/schema/validate_test.go)
TestValidate_AgainstRealExamplesIfPresent referenced
/Volumes/VPPD/projects/tracebloc/data-ingestors/examples/yaml,
which is one developer's macOS layout. Skipped gracefully when
absent but useless to anyone else.
Fix: read TRACEBLOC_INGESTORS_EXAMPLES env var; skip if unset.
Local dev opts in by exporting the var.
3. LOW — FormatErrors mutates caller's slice
(internal/schema/validate.go)
sort.Slice was called directly on the errs parameter, silently
reordering the caller's underlying array. Function name +
doc implied a pure formatter.
Fix: copy the slice header before sorting. Tiny allocation
relative to the schema validation itself. Added a regression
test pinning the input-preservation contract.
New regression tests pin all three contracts:
- TestIsSilentError covers the four shapes main() depends on
- TestFormatErrors_DoesNotMutateInput catches the side-effect
regression
Coverage moved up to 89.2% (internal/cli) and 80.7% (internal/schema).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@saadqbal
saadqbal merged commit f72cd79 into developMay 21, 2026
9 checks passed
LukasWodka added a commit that referenced this pull request Jul 8, 2026
…nreadable discovery
`tracebloc client create` could mint a NEW backend client and stamp the
cluster's cluster_id anchor onto it, even when a healthy DIFFERENT client was
already running on the cluster. The installer then refused to deploy the new
client (one-client-per-machine), leaving an orphaned "phantom" that owns the
anchor — so every later re-provision 409s ("cluster_conflict", mislabeled as
cross-account) and the real client can never reclaim the anchor. Confirmed in
the field: edge_device 1060 minted + anchored, never deployed, wedging a
cluster that actually runs 1044.
Root of the reproducible class: DiscoverInClusterClientID swallowed List/RBAC
errors into (nil, nil) — "nothing installed" — which is indistinguishable from
a genuinely fresh cluster, so runClientCreate fell through to a mint.
Fix (two edits):
- cluster/discover.go: DiscoverInClusterClientID is now three-valued. It
returns (nil, err) when it CANNOT determine — a deployments List error, a
secrets List error, or a release present whose CLIENT_ID is unreadable.
(nil, nil) now means only "reachable and genuinely no client". An empty
cluster still reports emptiness via an empty list, not an error, so fresh
installs are unaffected.
- cli/client.go: adoptLiveInClusterClient fails closed on a discovery error
when the cluster is REACHABLE (clusterID != "", i.e. the kube-system UID read
succeeded over the same kubeconfig) — refusing to mint a duplicate that could
strand the anchor. Only a genuinely unreachable cluster (clusterID == "",
where the UID read failed too) keeps the old fall-through to a non-anchored
mint (which stamps no anchor, so it can't orphan one) — the deliberate
headless/no-cluster path.
Tests (verified to FAIL against the pre-fix code):
- discover_test.go: DeploymentsListError / SecretsListError / ReleaseButNoSecret
now expect (nil, error).
- client_test.go: DiscoveryErrorReachableFailsClosed (reachable + discovery
error -> exitError, no mint) and DiscoveryErrorUnreachableMintsNonAnchored
(unreachable -> still mints, cluster_id empty) — the latter guards against
over-tightening into the legitimate headless path.
Full repo suite green; adversarially reviewed. This is fix#1 of the
phantom-client investigation (never mint over a live cluster). Follow-ups
(separate): backend same-account re-anchor + honest 409 message; orphan reaper.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
saadqbal pushed a commit that referenced this pull request Jul 9, 2026
…nreadable discovery (#190)
`tracebloc client create` could mint a NEW backend client and stamp the
cluster's cluster_id anchor onto it, even when a healthy DIFFERENT client was
already running on the cluster. The installer then refused to deploy the new
client (one-client-per-machine), leaving an orphaned "phantom" that owns the
anchor — so every later re-provision 409s ("cluster_conflict", mislabeled as
cross-account) and the real client can never reclaim the anchor. Confirmed in
the field: edge_device 1060 minted + anchored, never deployed, wedging a
cluster that actually runs 1044.
Root of the reproducible class: DiscoverInClusterClientID swallowed List/RBAC
errors into (nil, nil) — "nothing installed" — which is indistinguishable from
a genuinely fresh cluster, so runClientCreate fell through to a mint.
Fix (two edits):
- cluster/discover.go: DiscoverInClusterClientID is now three-valued. It
returns (nil, err) when it CANNOT determine — a deployments List error, a
secrets List error, or a release present whose CLIENT_ID is unreadable.
(nil, nil) now means only "reachable and genuinely no client". An empty
cluster still reports emptiness via an empty list, not an error, so fresh
installs are unaffected.
- cli/client.go: adoptLiveInClusterClient fails closed on a discovery error
when the cluster is REACHABLE (clusterID != "", i.e. the kube-system UID read
succeeded over the same kubeconfig) — refusing to mint a duplicate that could
strand the anchor. Only a genuinely unreachable cluster (clusterID == "",
where the UID read failed too) keeps the old fall-through to a non-anchored
mint (which stamps no anchor, so it can't orphan one) — the deliberate
headless/no-cluster path.
Tests (verified to FAIL against the pre-fix code):
- discover_test.go: DeploymentsListError / SecretsListError / ReleaseButNoSecret
now expect (nil, error).
- client_test.go: DiscoveryErrorReachableFailsClosed (reachable + discovery
error -> exitError, no mint) and DiscoveryErrorUnreachableMintsNonAnchored
(unreachable -> still mints, cluster_id empty) — the latter guards against
over-tightening into the legitimate headless path.
Full repo suite green; adversarially reviewed. This is fix#1 of the
phantom-client investigation (never mint over a live cluster). Follow-ups
(separate): backend same-account re-anchor + honest 409 message; orphan reaper.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
saadqbal pushed a commit that referenced this pull request Jul 9, 2026
…cluster_in_use (#191)
The CLI mapped every provisioning 409 to a static "registered to a different
tracebloc account — sign in to that account, or ask your admin" — which was
often FALSE (the same-account phantom case) and a dead end. With the backend's
fix#2 (backend#1021) the 409 body now distinguishes:
• cluster_conflict — genuinely another account; body carries owner_email;
• cluster_in_use — a same-account client is live on this cluster.
New conflictMessage() parses the 409 body and picks the right guidance:
• cross-account → "registered to another tracebloc account (<owner_email>) —
ask them to release it, or sign in as that account" (contact-the-owner, never
"delete the cluster" — it isn't ours to wipe; names the owner when supplied);
• cluster_in_use → "another tracebloc client (<name>) in your account is already
live on this cluster — offboard it first with `tracebloc delete`, or provision
on a separate machine".
Degrades gracefully against a backend without fix#2 (empty/unparseable body →
the generic cross-account text). The client-side not-owned refusal (no HTTP body)
keeps the generic message. Reworded crossAccountConflictMsg to match.
Companion to backend#1021 (fix#2) and cli#190 (fix#1) of the phantom-client
migration.
Tests: owner_email surfaced; cluster_in_use names the live client and does NOT
read as cross-account; existing cross-account + client-side-refusal messages
updated to the new wording. Full suite green; gofmt -s / errcheck / ineffassign /
misspell clean.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@saadqbal
saadqbal deleted the feat/149-schema-validator branch July 10, 2026 10:37
Sign up for freeto 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.

4 participants

@saadqbal@LukasWodka@shujaatTracebloc@aptracebloc