Skip to content

fix: address Bugbot findings from PR #22 (target_size order + Makefile/CI lint parity)#25

Merged
aptracebloc merged 1 commit into
developfrom
fix/bugbot-findings
Jun 3, 2026
Merged

fix: address Bugbot findings from PR #22 (target_size order + Makefile/CI lint parity)#25
aptracebloc merged 1 commit into
developfrom
fix/bugbot-findings

Conversation

@aptracebloc
Copy link
Copy Markdown
Contributor

@aptracebloc aptracebloc commented Jun 3, 2026

Summary

Addresses the two Cursor Bugbot findings raised on the develop → main sync PR #22. Per our flow, the fixes land on develop here; #22 then picks them up automatically and the Bugbot threads resolve.

1. target_size emitted as [width, height] — schema/ingestor want [height, width] (Medium)

The embedded ingest.v1.json documents target_size as [height, width] and the ingestor reads it that way, but buildImage wrote [width, height] (the order from --target-size WxH / DetectImageSize). Square datasets were unaffected — which is why it slipped through — but non-square datasets would pass local validation yet mis-validate resolution in the ingestor.

  • Swapped both emit sites (keypoint top-level + image spec.file_options) to [height, width]; kept the user-facing --target-size WxH flag and the auto-detect message unchanged.
  • Added TestBuild_TargetSize_EmittedHeightWidth — a non-square (640×480[480, 640]) regression test, since square sizes can't catch a swap.

2. make ci lint diverged from CI lint (Medium)

make lint ran golangci-lint, while CI's lint job runs errcheck + gofmt -s + ineffassign + misspell (golangci-lint-action is disabled pending #6). The Makefile exists specifically to keep local == CI.

Verification

make ci green end-to-end: go vet, go test -race, the new standalone lint, fmt-check, and schema-check (no drift).

🤖 Generated with Claude Code


Note

Medium Risk
Changes dataset push spec output for non-square resolutions (fixes ingestor mis-validation) and redefines local lint to match CI; both affect correctness and developer workflow but not auth or data paths directly.

Overview
Ingest spec target_size order: buildImage now emits [height, width] (schema/ingestor order) while SpecArgs.TargetSize stays [W, H] from --target-size / auto-detect. Both emit paths swap on output: keypoint top-level target_size and image spec.file_options.target_size. Comments on TargetSize document the storage vs emit convention.

Regression test: TestBuild_TargetSize_EmittedHeightWidth uses 640×480 so stored [640, 480] must become [480, 640] for image_classification and "and" keypoint_detection (square sizes would not catch a width/height swap).

Makefile / CI parity: make lint (part of make ci) runs the same standalone tools as .github/workflows/build.ymlerrcheck, ineffassign, and misspell via go run — instead of golangci-lint. make lint-full keeps the optional golangci-lint run pass until golangci-lint-action is re-enabled (#6).

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

1. target_size: emit [height, width] (was [width, height]) to match the ingest.v1 schema + ingestor; only affected non-square datasets. Added TestBuild_TargetSize_EmittedHeightWidth (non-square) regression test.

2. Makefile: make ci lint now runs the same standalone tools as CI (errcheck + ineffassign + misspell) instead of golangci-lint, restoring the local==CI invariant while golangci-lint-action stays disabled (#6); golangci-lint moved to make lint-full.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@LukasWodka
Copy link
Copy Markdown
Contributor

👋 Heads-up — Code review queue is at 13 / 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.)

@aptracebloc aptracebloc self-assigned this Jun 3, 2026
@aptracebloc aptracebloc merged commit 534d75b into develop Jun 3, 2026
13 checks passed
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