Uh oh!
There was an error while loading. Please reload this page.
feat(data ingest): expected image size + local min-size floor preview (#183) - #206
Merged
Conversation
…t-size as a check (#183) RFC-0002 §12.9. Mirror data-ingestors #348 in the CLI: image size is a CHECK, never a resize, and there is now a minimum-image-size floor the CLI previews locally so a below-floor dataset fails before the ingest, not after the upload in-cluster. - Schema re-sync: bump scripts/.data-ingestors-ref to the develop SHA that includes #348 and re-vendor internal/schema/ingest.v1.json, which now documents spec.file_options.min_size. Schema-drift check passes on the new ref. - Local floor preview: ValidateImages gains minW/minH and rejects any image with either side below the floor, naming the file, its dimensions, and the floor. PreflightDataset resolves the effective floor (spec.MinSize override, else push.MinImageSize = 32x32 — the exact mirror of ImageResolutionValidator.MIN_IMAGE_SIZE) exactly as the ingestor's __init__ defaults it. The floor takes precedence over a target_size mismatch, matching #348. - --min-size WxH flag plumbs to spec.file_options.min_size (mirrors how --target-size plumbs); the preview uses the effective value. - --target-size copy reframed (flag + interactive prompt): it is the size images must ALREADY be — tracebloc never resizes — and names the min-size floor. - Parity harness: fixtures resized above the 32px floor so each case still tests its intended rule (not the new floor), a dedicated imgc-too-small case added, and goldens regenerated against the pinned ref with the REAL validators. The re-sync also pulled in #338 (the in-cluster BOM-probe fix, which merges below #348), so tabular-bom is now a documented divergence: the pinned ingestor accepts, but the CLI keeps previewing the rejection because the deployed ingestor (v0.5.7) predates #338. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…+ close the min-size parity gap (#183) Review follow-ups on the #183 min-image-size work: - Deployment parity (was the inverse of the tabular-BOM handling): the 32x32 floor lives in data-ingestors only on develop (di#348/#356); the DEPLOYED ingestor (v0.5.7/v0.6.0) has no floor and ingests small images fine. The preview no longer applies the default floor on its own — a default block would exit-3 an ingest the live cluster accepts. It now previews the floor ONLY when the customer set --min-size (their own declared requirement). Flip imgc-too-small to a flagged deliberate divergence; restore default-on once di#348 reaches prod. - Message: the too-small reject no longer asserts "the cluster rejects images below this size" (false for the deployed cluster) — it speaks to the floor the customer set with --min-size. - Flag help: --min-size no longer calls 32x32 "the smallest tracebloc will ingest" (only-raise), which contradicted the reject's "lower the floor with --min-size" and the feature (16x16 is accepted end-to-end). Reworded to raise-or-lower with an honest "no local size check" default. - Parity harness: min_size was representable on neither side, so the headline override was cross-checked by neither. Plumb it through the Go parityCase + SpecArgs and gen-validator-goldens.py, and add imgc-min-size-override — 24x40 images with an explicit [16,32] floor that the DEFAULT 32x32 would reject, pinning the override is read (not the default) and its [W,H] orientation is honored, on both sides. Goldens regenerated against the pinned ref (efaeb07); only the new case is added, imgc-too-small's golden is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e --min-size (#206 review) Follow-ups from the #206 code review: - --target-size / --min-size are image-only, but were parsed only inside the image branch, so on a tabular/text task the flag value (even a malformed one) was silently dropped. Reject them explicitly at the category gate with an actionable exit-2 message. Adds a table-driven test. - renderReview now echoes a set --min-size in the interactive confirm summary, so a mistyped floor is caught before touching data. - Drop dangling citations in the MinImageSize doc-comment ("RFC-0002 §12.9" / "Principle 6" — neither exists; RFC-0002 has 5 principles and §12 stops at §12.8). Keep the real source (data-ingestors #348). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The test used LocalPath "./x"; on the branch tip the guard fired before any path check, but CI runs the PR merged into develop, which added an earlier dataset-path stat (#181) that rejects a nonexistent path with exit 3 before the guard's exit 2. Point LocalPath at a t.TempDir() so the stat passes and the image-only guard is what fires. Verified against a local merge with origin/develop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
approved these changes
Jul 10, 2026
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 freeto 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
Two image-preflight improvements for
tracebloc data ingest, so a customer finds out about size problems before the ingest, not after (their data never leaves the box either way):--target-sizeis reframed as a check, not a resize. tracebloc validates every image is exactly that size and rejects any that differ; the CLI previews the same reject locally. Default: read from your first image.--min-size WxHpreviews data-ingestors' minimum-image-size floor (ci: bump GitHub Actions (bundles #342–#346, matches artifact majors) #348) locally. It mirrors the ingestor's_meets_min_sizeexactly: too small when either side is below the floor, boundary inclusive,[W, H]order, no swap. The override travels asspec.file_options.min_size.Closes#183.
What changed
internal/cli/data.go—--target-size/--min-sizeflags + help; plumb--min-sizetospec.file_options.min_size.internal/push/spec.go—SpecArgs.MinSize,MinImageSizeconstant (the mirror of the upstream default), emit underfile_options.internal/push/preflight.go—ValidateImagespreviews the floor (floor-first, then the resolution mismatch, matching the ingestor's error order);PreflightDatasetwiring.internal/schema/ingest.v1.json— re-synced to the pinned refefaeb07, which learned aboutmin_size.parity_golden_test.go,scripts/gen-validator-goldens.py,testdata/parity/) —min_sizeis now representable on both sides, with a new end-to-end case.Review follow-ups folded in (last commit)
--min-size(their own declared requirement).imgc-too-smallis now a flagged deliberate divergence.--min-sizeno longer calls 32x32 "the smallest tracebloc will ingest" (which contradicted both the reject's "lower the floor with--min-size" and the feature — 16x16 is accepted end-to-end). Reworded to raise-or-lower, honest "no local size check" default.min_sizewas representable on neither side of the harness, so the headline override was cross-checked by neither. Addedimgc-min-size-override: 24x40 images with an explicit[16,32]floor that the default 32x32 would reject — pinning that the override is actually read (not silently the default) and its[W, H]orientation is honored, on both the CLI and the real ingestor.Test plan
Local, all green:
go build ./...go test -race -cover ./...(incl.TestValidatorParity,TestValidateImagesMinSize,TestBuild_WithMinSize_PassesSchema)gofmt -l ./gofmt -s -l .→ emptygo vet ./..../scripts/sync-schema.sh --check→ no drift (pinned refefaeb07)./scripts/coverage-floor.sh→ floors metimgc-min-size-overrideadded; every existing golden byte-stable (incl.imgc-too-small→ reject).Decisions to ratify
MinImageSize = {32, 32}, either-side / boundary-inclusive /[W, H]. It is the CLI's mirror ofImageResolutionValidator.MIN_IMAGE_SIZE; do not invent a different floor here (Principle 6).--min-size; the default 32x32 is not previewed, because the deployed ingestor has none. When di#348 ships to prod, defaultPreflightDatasettoMinImageSizeand flipimgc-too-smallback toreject(both flagged inline + incases.json).internal/schema/ingest.v1.jsonis pinned toefaeb07(viascripts/.data-ingestors-ref), the ref that addedmin_size. Adopting further upstream changes stays a deliberate SHA bump + re-sync.🤖 Generated with Claude Code
Note
Medium Risk
Touches image preflight and ingest spec emission with deployment-parity caveats; behavior is heavily tested via parity goldens but wrong floor defaults could block or allow bad ingests until prod catches up with #348.
Overview
Adds
--min-size WxHfor image ingest tasks so customers can set a minimum resolution floor locally and in the emitted spec (spec.file_options.min_size), aligned with data-ingestors #348.--target-sizehelp and interactive copy now stress that tracebloc validates size and never resizes.Local preflight extends
ValidateImageswith an optional min-size check (either dimension below the floor fails; floor errors beat target-size mismatches). The preview applies that floor only when--min-sizeis set, not the upstream 32×32 default, so the CLI does not reject ingests that today’s deployed ingestor still accepts without a floor.data ingestrejects--target-size/--min-sizeon non-image tasks at exit 2 instead of silently ignoring them.ParseMinSizeshares WxH parsing with target size;SpecArgs.MinSizeand schema re-sync (min_sizeonfile_options, pinned ingestors ref) wire throughBuild. Parity fixtures and goldens were updated (including deliberate CLI vs pinned-ingestor divergences for default floor and tabular BOM).Reviewed by Cursor Bugbot for commit fd4688e. Bugbot is set up for automated code reviews on this repo. Configure here.