Skip to content

ci: flip deadcode gate to blocking; delete the 3 dead #127 leftovers - #313

Merged
LukasWodka merged 3 commits into
ci/279-staticcheck-pinnedfrom
ci/281-deadcode-blocking
Jul 14, 2026
Merged

ci: flip deadcode gate to blocking; delete the 3 dead #127 leftovers#313
LukasWodka merged 3 commits into
ci/279-staticcheck-pinnedfrom
ci/281-deadcode-blocking

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

WS-A.4 of epic tracebloc/backend#1106: the deadcode gate goes blocking, the three dead #127 leftovers are deleted, and the four legit findings get an explicit, reasoned allowlist.

Key mechanism note: x/tools/cmd/deadcodealways exits 0, even with findings — so the old advisory step could never have been flipped by just removing continue-on-error. The gate now runs scripts/deadcode-check.sh, which normalizes findings to <file>: <func> (line numbers stripped so ordinary edits don't red the gate), subtracts scripts/deadcode-allowlist.txt, and fails on anything left. Stale allowlist entries warn without failing. Both loops use it: build.yml's deadcode step (continue-on-error removed) and make deadcode (|| true removed); the pin stays v0.48.0, overridable via DEADCODE_VERSION.

Deleted (each re-verified dead with the pinned deadcode@v0.48.0 before deletion)

FuncDisposition
config.clearAllDeleted + its 3 dedicated tests (TestClearAll, TestClearAll_HomeError, TestClear) — logout uses Save; nothing production ever called it
push.allCategoryIDsDeleted from category.go; moved verbatim into category_registry_test.go — the registry-pinning tests (cli#74 drift guards) legitimately iterate every id, but the shipped binary shouldn't carry a test-only helper
submit.isSubmitErrorDeleted from client.go (+ now-orphaned errors import); moved into client_test.go as the test-local assertion helper it always was (used by client_test + submit_test, same package)

Allowlisted (the 4 legit findings, with reasons in the file)

  • Status.String, JobOutcome.String — Stringers reached only via fmt's reflection dispatch, invisible to static reachability
  • ReadLabelValues, inferColumnType — di#349 test-only parity harnesses that must live in production source

Stacking note

Stacked on #304 (goimports/Dependabot, WS-A.3), which stacks on #302 (staticcheck, WS-A.2) — merge order #302#304 → this; each then shows only its own diff. Shared-file edits (build.yml lint job, Makefile) are surgical — expected trivial rebase vs sibling PRs touching these files, merge order flexible.

Test plan (ran locally on this branch)

  • ./scripts/deadcode-check.sh — clean (4 allowlisted); negative path verified: removing an allowlist entry makes it exit 1 naming the finding
  • make deadcode (blocking path through the Makefile) — green
  • go build ./...; go test -race ./internal/config/... ./internal/push/... ./internal/submit/... — green
  • scripts/coverage-floor.sh still clears after the test deletions (cli 82.9% ≥ 68, submit 80.3% ≥ 72)
  • staticcheck (all,-ST1005) still clean; make fmt-check (gofmt + goimports) green; build.yml YAML-parses; shellcheck clean on the new script
  • Not run locally: full 8-platform matrix, installer harness, e2e — CI covers these

Fixes#281

🤖 Generated with Claude Code


Note

Low Risk
Changes are CI/tooling and removal of unused code with no production callers; behavior of login/config and submit paths is unchanged.

Overview
Deadcode goes blocking in CI (build.yml) and make deadcode: advisory continue-on-error / || true is removed in favor of scripts/deadcode-check.sh, which parses deadcode output (the tool always exits 0), subtracts scripts/deadcode-allowlist.txt, and fails on any other unreachable-from-./cmd/tracebloc symbol. Four known false positives (Stringers + di#349 parity helpers) stay allowlisted with documented reasons; stale allowlist entries only warn.

Production dead code cleanup (#127):config.clearAll and its dedicated tests are deleted (logout uses Save). push.allCategoryIDs and submit.isSubmitError move into their package test files so the binary no longer ships test-only helpers.

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

@LukasWodka
LukasWodka requested a review from saadqbalJuly 14, 2026 12:35
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

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

  • averaging-service#181 — feat(weights): normalize-on-read — mixed cycles average instead of rejecting (SEC-03 §8 step 2) · author: @shujaatTracebloc · no reviewer assigned
  • averaging-service#182 — feat(weights): averaging writes SafeTensors (TF + PyTorch) — SEC-03 §8 step 4 · author: @shujaatTracebloc · no reviewer assigned
  • backend#1079 — feat(global_meta): edge dataset_meta exposure + attributes contract at ingest (#924 G4a) · author: @divyasinghds · no reviewer assigned
  • backend#1086 — docs(rfc): SafeTensors weight-format migration — SEC-03 Phase 1 (RFC 0004) · author: @shujaatTracebloc · no reviewer assigned
  • backend#1093 — chore(deps): bump django from 5.2.14 to 5.2.15 · author: @dependabot · no reviewer assigned
  • backend#1095 — feat(experiment): configurable preprocessing knobs incl. tabular scaler — RFC 0003 L1 + L1b (#1094) · author: @LukasWodka · no reviewer assigned
  • backend#1100 — fix(boot): pin SDK install to tracebloc==0.11.2, drop 404 dev line (#1098) · author: @LukasWodka · reviewer: @saqlainsyed007
  • backend#1105 — perf(api): query micro-fixes — notifications N+1, cached data_scientist, composite index, sampling (#975) · author: @aptracebloc · no reviewer assigned
  • backend#1107 — perf(experiments): prefetch ExperimentListSerializer relations + cache carbon intensity (#977) · author: @aptracebloc · reviewer: @saadqbal
  • backend#1108 — test(api): CLI response-shape contract fixtures for the 9 CLI endpoints (WS-D.2) · author: @LukasWodka · reviewer: @saadqbal

Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.)

@LukasWodka
LukasWodka changed the base branch from develop to ci/280-goimports-dependabotJuly 14, 2026 12:49
@LukasWodkaLukasWodka self-assigned this Jul 14, 2026
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

@BugBot run

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit cb40e76. Configure here.

@saadqbalsaadqbal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍 Verified the 3 deletions are entrypoint-unreachable (test-only refs, rewired here) — and confirmed clearAll was genuinely orphaned: logout clears via its own path (profile clear + Save prune), not clearAll. All builds + Test green. The blocking gate design (strip line:col, subtract allowlist, stale entries warn-not-fail) is solid. (Stacked on #304.)

@saadqbal

Copy link
Copy Markdown
Collaborator

Status: approved, stacked on #304 (ci/280-goimports-dependabot). Lands after #302#304. Nothing needed here yet.

LukasWodkaand others added 2 commits July 14, 2026 17:13
…monthly)
goimports -local github.com/tracebloc/cli now blocks in both loops:
build.yml's lint job (pinned goimports@v0.48.0, same x/tools version as
the deadcode pin) and the Makefile's fmt-check target, with make fmt
extended to auto-fix. .golangci.yml already declared this grouping via
local-prefixes but nothing enforced it — 4 files had drifted (data.go,
data_test.go, ingestion_run_test.go, resources_set_test.go), fixed here
with import-grouping-only diffs.
.github/dependabot.yml extends the org's backend-only Dependabot pilot:
gomod weekly (k8s.io/* + sigs.k8s.io/* grouped, golang.org/x/* grouped),
github-actions monthly. Unlike backend's security-only config, the CLI
takes real version updates — customers install this binary, so staying
current is security posture (see #276). Org-wide rollout decision
flagged to Asad on the PR.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Regroups imports into the layout goimports -local github.com/tracebloc/cli
emits (stdlib / third-party / tracebloc-local). gofmt and goimports only
sort within existing blank-line groups, so the manual groups passed
fmt-check as-is but were not the canonical single-block output; Bugbot and
our precheck both flagged the divergence. Verified idempotent under the
pinned goimports v0.48.0 and green on make fmt-check + go build ./...
Rebased onto ci/279 (post-#303 data.go split): data.go's import block is
already canonical from the #303 split, so its canonicalization here is a
no-op and dropped — this commit now regroups data_test.go only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LukasWodka
LukasWodkaforce-pushed the ci/280-goimports-dependabot branch from 7825106 to 6f34128CompareJuly 14, 2026 15:15
The deadcode CI step loses continue-on-error (and the Makefile target
its '|| true'): both now run scripts/deadcode-check.sh, which fails on
any function unreachable from ./cmd/tracebloc that isn't declared in
scripts/deadcode-allowlist.txt. The tool itself always exits 0, so the
old advisory step never could have blocked — the gate keys on output.
Deleted (verified still dead with the pinned deadcode@v0.48.0):
- config.clearAll + its three dedicated tests (TestClearAll,
TestClearAll_HomeError, TestClear) — logout uses Save, nothing else
ever called it
- push.allCategoryIDs — moved verbatim into category_registry_test.go
(the registry-pinning tests legitimately iterate every id; the
shipped binary shouldn't carry the helper)
- submit.isSubmitError — moved into client_test.go as the test-local
assertion helper it always was (orphaned 'errors' import dropped)
Allowlisted with reasons (the 4 legit findings): Status.String +
JobOutcome.String (fmt-reflection Stringers) and ReadLabelValues +
inferColumnType (di#349 test-only parity harnesses). Stale allowlist
entries warn without failing; line numbers are stripped so edits that
shift code don't red the gate.
Coverage floors still clear after the test deletions (cli 82.9% >= 68,
submit 80.3% >= 72).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LukasWodka
LukasWodkaforce-pushed the ci/281-deadcode-blocking branch from cb40e76 to c556c9cCompareJuly 14, 2026 15:18
Base automatically changed from ci/280-goimports-dependabot to ci/279-staticcheck-pinnedJuly 14, 2026 15:22
@LukasWodka
LukasWodka merged commit 9470567 into ci/279-staticcheck-pinnedJul 14, 2026
6 checks passed
@LukasWodka
LukasWodka deleted the ci/281-deadcode-blocking branch July 14, 2026 16:24
LukasWodka added a commit that referenced this pull request Jul 14, 2026
…313)
* ci: goimports -local gate + Dependabot config (gomod weekly, actions monthly)
goimports -local github.com/tracebloc/cli now blocks in both loops:
build.yml's lint job (pinned goimports@v0.48.0, same x/tools version as
the deadcode pin) and the Makefile's fmt-check target, with make fmt
extended to auto-fix. .golangci.yml already declared this grouping via
local-prefixes but nothing enforced it — 4 files had drifted (data.go,
data_test.go, ingestion_run_test.go, resources_set_test.go), fixed here
with import-grouping-only diffs.
.github/dependabot.yml extends the org's backend-only Dependabot pilot:
gomod weekly (k8s.io/* + sigs.k8s.io/* grouped, golang.org/x/* grouped),
github-actions monthly. Unlike backend's security-only config, the CLI
takes real version updates — customers install this binary, so staying
current is security posture (see #276). Org-wide rollout decision
flagged to Asad on the PR.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* ci: canonicalize import grouping in data_test.go for the goimports gate
Regroups imports into the layout goimports -local github.com/tracebloc/cli
emits (stdlib / third-party / tracebloc-local). gofmt and goimports only
sort within existing blank-line groups, so the manual groups passed
fmt-check as-is but were not the canonical single-block output; Bugbot and
our precheck both flagged the divergence. Verified idempotent under the
pinned goimports v0.48.0 and green on make fmt-check + go build ./...
Rebased onto ci/279 (post-#303 data.go split): data.go's import block is
already canonical from the #303 split, so its canonicalization here is a
no-op and dropped — this commit now regroups data_test.go only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* ci: flip deadcode gate to blocking; delete the 3 dead #127 leftovers
The deadcode CI step loses continue-on-error (and the Makefile target
its '|| true'): both now run scripts/deadcode-check.sh, which fails on
any function unreachable from ./cmd/tracebloc that isn't declared in
scripts/deadcode-allowlist.txt. The tool itself always exits 0, so the
old advisory step never could have blocked — the gate keys on output.
Deleted (verified still dead with the pinned deadcode@v0.48.0):
- config.clearAll + its three dedicated tests (TestClearAll,
TestClearAll_HomeError, TestClear) — logout uses Save, nothing else
ever called it
- push.allCategoryIDs — moved verbatim into category_registry_test.go
(the registry-pinning tests legitimately iterate every id; the
shipped binary shouldn't carry the helper)
- submit.isSubmitError — moved into client_test.go as the test-local
assertion helper it always was (orphaned 'errors' import dropped)
Allowlisted with reasons (the 4 legit findings): Status.String +
JobOutcome.String (fmt-reflection Stringers) and ReadLabelValues +
inferColumnType (di#349 test-only parity harnesses). Stale allowlist
entries warn without failing; line numbers are stripped so edits that
shift code don't red the gate.
Coverage floors still clear after the test deletions (cli 82.9% >= 68,
submit 80.3% >= 72).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
LukasWodka added a commit that referenced this pull request Jul 14, 2026
…313)
* ci: goimports -local gate + Dependabot config (gomod weekly, actions monthly)
goimports -local github.com/tracebloc/cli now blocks in both loops:
build.yml's lint job (pinned goimports@v0.48.0, same x/tools version as
the deadcode pin) and the Makefile's fmt-check target, with make fmt
extended to auto-fix. .golangci.yml already declared this grouping via
local-prefixes but nothing enforced it — 4 files had drifted (data.go,
data_test.go, ingestion_run_test.go, resources_set_test.go), fixed here
with import-grouping-only diffs.
.github/dependabot.yml extends the org's backend-only Dependabot pilot:
gomod weekly (k8s.io/* + sigs.k8s.io/* grouped, golang.org/x/* grouped),
github-actions monthly. Unlike backend's security-only config, the CLI
takes real version updates — customers install this binary, so staying
current is security posture (see #276). Org-wide rollout decision
flagged to Asad on the PR.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* ci: canonicalize import grouping in data_test.go for the goimports gate
Regroups imports into the layout goimports -local github.com/tracebloc/cli
emits (stdlib / third-party / tracebloc-local). gofmt and goimports only
sort within existing blank-line groups, so the manual groups passed
fmt-check as-is but were not the canonical single-block output; Bugbot and
our precheck both flagged the divergence. Verified idempotent under the
pinned goimports v0.48.0 and green on make fmt-check + go build ./...
Rebased onto ci/279 (post-#303 data.go split): data.go's import block is
already canonical from the #303 split, so its canonicalization here is a
no-op and dropped — this commit now regroups data_test.go only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* ci: flip deadcode gate to blocking; delete the 3 dead #127 leftovers
The deadcode CI step loses continue-on-error (and the Makefile target
its '|| true'): both now run scripts/deadcode-check.sh, which fails on
any function unreachable from ./cmd/tracebloc that isn't declared in
scripts/deadcode-allowlist.txt. The tool itself always exits 0, so the
old advisory step never could have blocked — the gate keys on output.
Deleted (verified still dead with the pinned deadcode@v0.48.0):
- config.clearAll + its three dedicated tests (TestClearAll,
TestClearAll_HomeError, TestClear) — logout uses Save, nothing else
ever called it
- push.allCategoryIDs — moved verbatim into category_registry_test.go
(the registry-pinning tests legitimately iterate every id; the
shipped binary shouldn't carry the helper)
- submit.isSubmitError — moved into client_test.go as the test-local
assertion helper it always was (orphaned 'errors' import dropped)
Allowlisted with reasons (the 4 legit findings): Status.String +
JobOutcome.String (fmt-reflection Stringers) and ReadLabelValues +
inferColumnType (di#349 test-only parity harnesses). Stale allowlist
entries warn without failing; line numbers are stripped so edits that
shift code don't red the gate.
Coverage floors still clear after the test deletions (cli 82.9% >= 68,
submit 80.3% >= 72).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
LukasWodka added a commit that referenced this pull request Jul 14, 2026
…findings (#302)
* ci: re-enable staticcheck (pinned standalone) + fix the 5 non-ST1005 findings
Pinned staticcheck@2025.1.1 joins the standalone lint set in both the
Makefile lint target and build.yml's lint job (-checks all,-ST1005),
mirroring the errcheck/ineffassign pin pattern. The golangci-lint OOM
story that originally disabled it is stale for the standalone binary:
a full run is ~12s wall locally.
Findings fixed (the full non-ST1005 set):
- ST1008: runSet (resources_set_test) returns (string, error), error last
- ST1003: CheckMaskIdColumn -> CheckMaskIDColumn (+ call sites/comments)
- ST1003: errors_as -> errorsAs, io_eof_or_similar -> ioEOFOrSimilar
- ST1020: CoreFloorText doc comment starts with the function name
ST1005 stays excluded: it flags ~58 customer-visible error strings that
need a wording review, not a mechanical sweep — separate follow-up.
Rebased onto develop: union-merged the lint job with develop's govulncheck
job (#278), coverage-floor step (#301) and deadcode advisory gate; kept the
audit comment #286 added above CheckMaskID*Column. The 'going back to the
action' comment in build.yml is KEPT (not deleted) — #6 is still open on
develop, so golangci-lint-action stays disabled and that rationale still
holds. Detached the four data_ingest_*.go header comments (#303 split) plus
exitcodes.go's (#284, which develop merged after this branch was cut) from
the package clause so the newly-enabled staticcheck gate is ST1000-clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* ci: goimports -local gate + Dependabot config (gomod weekly, actions monthly) (#304)
* ci: goimports -local gate + Dependabot config (gomod weekly, actions monthly)
goimports -local github.com/tracebloc/cli now blocks in both loops:
build.yml's lint job (pinned goimports@v0.48.0, same x/tools version as
the deadcode pin) and the Makefile's fmt-check target, with make fmt
extended to auto-fix. .golangci.yml already declared this grouping via
local-prefixes but nothing enforced it — 4 files had drifted (data.go,
data_test.go, ingestion_run_test.go, resources_set_test.go), fixed here
with import-grouping-only diffs.
.github/dependabot.yml extends the org's backend-only Dependabot pilot:
gomod weekly (k8s.io/* + sigs.k8s.io/* grouped, golang.org/x/* grouped),
github-actions monthly. Unlike backend's security-only config, the CLI
takes real version updates — customers install this binary, so staying
current is security posture (see #276). Org-wide rollout decision
flagged to Asad on the PR.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* ci: canonicalize import grouping in data_test.go for the goimports gate
Regroups imports into the layout goimports -local github.com/tracebloc/cli
emits (stdlib / third-party / tracebloc-local). gofmt and goimports only
sort within existing blank-line groups, so the manual groups passed
fmt-check as-is but were not the canonical single-block output; Bugbot and
our precheck both flagged the divergence. Verified idempotent under the
pinned goimports v0.48.0 and green on make fmt-check + go build ./...
Rebased onto ci/279 (post-#303 data.go split): data.go's import block is
already canonical from the #303 split, so its canonicalization here is a
no-op and dropped — this commit now regroups data_test.go only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* ci: flip deadcode gate to blocking; delete the 3 dead #127 leftovers (#313)
* ci: goimports -local gate + Dependabot config (gomod weekly, actions monthly)
goimports -local github.com/tracebloc/cli now blocks in both loops:
build.yml's lint job (pinned goimports@v0.48.0, same x/tools version as
the deadcode pin) and the Makefile's fmt-check target, with make fmt
extended to auto-fix. .golangci.yml already declared this grouping via
local-prefixes but nothing enforced it — 4 files had drifted (data.go,
data_test.go, ingestion_run_test.go, resources_set_test.go), fixed here
with import-grouping-only diffs.
.github/dependabot.yml extends the org's backend-only Dependabot pilot:
gomod weekly (k8s.io/* + sigs.k8s.io/* grouped, golang.org/x/* grouped),
github-actions monthly. Unlike backend's security-only config, the CLI
takes real version updates — customers install this binary, so staying
current is security posture (see #276). Org-wide rollout decision
flagged to Asad on the PR.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* ci: canonicalize import grouping in data_test.go for the goimports gate
Regroups imports into the layout goimports -local github.com/tracebloc/cli
emits (stdlib / third-party / tracebloc-local). gofmt and goimports only
sort within existing blank-line groups, so the manual groups passed
fmt-check as-is but were not the canonical single-block output; Bugbot and
our precheck both flagged the divergence. Verified idempotent under the
pinned goimports v0.48.0 and green on make fmt-check + go build ./...
Rebased onto ci/279 (post-#303 data.go split): data.go's import block is
already canonical from the #303 split, so its canonicalization here is a
no-op and dropped — this commit now regroups data_test.go only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* ci: flip deadcode gate to blocking; delete the 3 dead #127 leftovers
The deadcode CI step loses continue-on-error (and the Makefile target
its '|| true'): both now run scripts/deadcode-check.sh, which fails on
any function unreachable from ./cmd/tracebloc that isn't declared in
scripts/deadcode-allowlist.txt. The tool itself always exits 0, so the
old advisory step never could have blocked — the gate keys on output.
Deleted (verified still dead with the pinned deadcode@v0.48.0):
- config.clearAll + its three dedicated tests (TestClearAll,
TestClearAll_HomeError, TestClear) — logout uses Save, nothing else
ever called it
- push.allCategoryIDs — moved verbatim into category_registry_test.go
(the registry-pinning tests legitimately iterate every id; the
shipped binary shouldn't carry the helper)
- submit.isSubmitError — moved into client_test.go as the test-local
assertion helper it always was (orphaned 'errors' import dropped)
Allowlisted with reasons (the 4 legit findings): Status.String +
JobOutcome.String (fmt-reflection Stringers) and ReadLabelValues +
inferColumnType (di#349 test-only parity harnesses). Stale allowlist
entries warn without failing; line numbers are stripped so edits that
shift code don't red the gate.
Coverage floors still clear after the test deletions (cli 82.9% >= 68,
submit 80.3% >= 72).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.

2 participants

@LukasWodka@saadqbal