chore: sweep dead code + close module/ lint-gate gap by trilamsr · Pull Request #486 · TraceCoreAI/tracecore

trilamsr · 2026-06-02T05:01:29Z

Summary

Wave-3 refactor accumulation (multiple detectors hit NORTHSTAR, RFC-0013 deletions cascading, 23+ PRs merged) left dead Go code in module/ that no linter was catching. This PR deletes it and closes the CI gap that allowed it to accumulate.

Deleted 3 unused Go functions + 1 unused shell var (-39 lines source, +19 lines gate config).
Added make lint-unused-module and wired it into make check so future module/ unused-symbol accumulation fails pre-commit.

Root cause (the lint-gate gap)

make lint runs go tool golangci-lint run ./... from repo root. Go workspace mode resolves ./... per-module, and module/ is a separate Go module (under go.work). So the in-repo submodule was never linted despite unused being enabled in .golangci.yml. Three unused functions accumulated through that blind spot since the detectors landed.

Deletions

module/pkg/patterns/pod_evicted.go::displayPodName — package-private (lowercase), zero callers. Comment falsely claimed "external callers, e.g. xid_correlation, may use it" — xid_correlation.go only mentions it in a doc comment about a different helper (podDisplayLength). Hot path inlines the logic into renderHeadline. (-19 lines)
module/pkg/patterns/pod_evicted.go::formatTimestamp — package-private, zero callers. Comment falsely claimed "retained for external callers (test helpers)" — none exist. Hot path uses appendTimestamp directly to share a buffer. (-10 lines)
module/receiver/ncclfrreceiver/nccl_fr_test.go::logsSink.first — test helper, zero callers across the package. (-9 lines)
scripts/validator-recipe.sh::EXAMPLES_DIR — assigned, never read. (-1 line)

CI gate (the fix, not just a workaround)

New make lint-unused-module target runs golangci-lint --no-config --default=none --enable=unused against module/. Wired into check (pre-commit aggregate gate). Scoped to unused only because the broader module/ lint pass surfaces ~199 pre-existing findings across errorlint, exhaustive, forcetypeassert, goconst, gocyclo, gosec, perfsprint, prealloc, predeclared, revive, staticcheck, testifylint, wrapcheck — those need their own dedicated cleanup PRs. Filed-as-followup rather than bundled here to keep this PR reviewable.

--no-config is required: the repo .golangci.yml v2 schema enables a broad linter set; without --no-config that set wins over the --enable=unused filter (validated empirically — the --default=none flag without --no-config still produced 199 findings).

Validation that the gate would have caught this

Stashed only the source deletions (keeping the Makefile change) and re-ran make lint-unused-module:

pkg/patterns/pod_evicted.go:383:6: func displayPodName is unused (unused)
pkg/patterns/pod_evicted.go:414:6: func formatTimestamp is unused (unused)
receiver/ncclfrreceiver/nccl_fr_test.go:456:20: func (*logsSink).first is unused (unused)
3 issues:
* unused: 3
make: *** [lint-unused-module] Error 1

All three U1000 hits flagged → exit non-zero. Future regressions of this exact class will fail pre-commit.

Out of scope (filed as follow-up)

The ~199 pre-existing module/ lint findings (non-unused linters). Promoting module/ to full make lint coverage is a multi-PR cleanup; the cut-off here is "stop new dead code from landing", not "fix every legacy module/ issue."

chore: delete 3 unused functions + 1 unused shell var from the wave-3 detector refactor; close the `make lint` gap that left `module/` unlinted (workspace-mode `./...` is module-scoped, so the in-repo submodule was silently skipped) by adding a `lint-unused-module` gate wired into pre-commit `check`.

Test plan

make check — green (lint + lint-unused-module + vet + mod-verify + attribute-namespace-check)
cd module && go test -race -count=1 ./... — all packages ok
cd module && go test -race -count=1 -tags=alldeps ./... — green (replay + selftel + patterns + receiver all pass)
go test -race -count=1 ./... — root tests green (bench/detectors, bench/overhead, components, tools)
staticcheck -checks=U1000 ./module/... — 0 issues post-delete (was 3 pre-delete)
go test -run='^$' -bench=BenchmarkPodEvictedDetector -benchtime=1x ./bench/detectors/ — pod-evicted benchmark still runs (didn't touch the hot path)
Gate validated against pre-deletion code: stashed deletions, re-ran make lint-unused-module → all 3 U1000 hits flagged + exit non-zero

Wave-3 refactor accumulation left three unused Go functions and one unused shell var across module/ and scripts/. Root cause: the repo `.golangci.yml` enables `unused` (U1000) but `make lint` runs `golangci-lint run ./...` from repo root only — workspace mode resolves `./...` per-module, so the in-repo `module/` submodule was never linted. Three accumulated unused symbols since detectors hit NORTHSTAR landed. Deletions: * `module/pkg/patterns/pod_evicted.go::displayPodName` — comment claimed "external callers, e.g. xid_correlation, may use it" but the helper is package-private (lowercase) and never referenced; hot path inlines the logic. (-19 lines) * `module/pkg/patterns/pod_evicted.go::formatTimestamp` — comment claimed "retained for external callers (test helpers)" but the function is package-private and unused; hot path uses `appendTimestamp` which shares the buffer. (-10 lines) * `module/receiver/ncclfrreceiver/nccl_fr_test.go::logsSink.first` — test helper with no callers anywhere in the package. (-9 lines) * `scripts/validator-recipe.sh::EXAMPLES_DIR` — assigned, never read. (-1 line) CI gate (new): `make lint-unused-module` runs `golangci-lint --no-config --default=none --enable=unused` against `module/` and is wired into the `check` (pre-commit) gate. Scoped to `unused` only because the broader module/ lint pass surfaces ~199 pre-existing findings (errorlint, exhaustive, forcetypeassert, …) that need their own dedicated cleanup PRs — bundling them here would balloon scope. Validated: with the deleted symbols restored, the new gate exits non-zero and prints all three U1000 hits. Verification: * `make check` — green (lint + lint-unused-module + vet + mod-verify + attribute-namespace-check) * `cd module && go test -race -count=1 ./...` — all pkgs ok * `cd module && go test -race -count=1 -tags=alldeps ./...` — green * `go test -race -count=1 ./...` — root tests green * `staticcheck -checks=U1000 ./module/...` — 0 issues post-delete * `bench/detectors BenchmarkPodEvictedDetector` — runs (untouched) Signed-off-by: Tri Lam <trilamsr@gmail.com>

trilamsr · 2026-06-02T05:19:31Z

Independent Adversarial Review: PR #486

Success Criteria Audit

✅ All 4 deletions are zero-callers (verified via git grep)
✅ Root cause (workspace-mode ./... scoping) is accurate
✅ Lint gate properly scoped to unused only, pre-existing 199 findings deferred
✅ Gate wired into check target correctly
🔵 Comment density excessive (needs simplification)
❓ Follow-up issue missing (blocking)

Findings

Makefile, lines 18–33 (lint-unused-module target)
🔵 nit: Comment is 14 lines for 1-line command. Root-cause explanation + --no-config rationale are correct, but could fit in 4–5 lines. The branch-name mention (chore/dead-code-sweep-2026-06) and "199+ pre-existing findings" should live in a filed follow-up issue, not inline Makefile comments.

Makefile, line 43 (check target)
✅ Correct: lint-unused-module properly wired into pre-commit aggregate gate; order (after lint, before vet) is sensible.

module/pkg/patterns/pod_evicted.go (displayPodName/formatTimestamp)
🟡 risk: Comment on displayPodName became stale after PR #448 merged. The comment claimed "external callers, e.g. xid_correlation, may use it" — but xid_correlation was refactored in PR #448 (18:58 UTC June 1) to use podDisplayLength instead. By the time PR #456 merged later the same day (19:43 UTC), the comment was false. This PR's deletion is correct; the stale comment is a pre-existing wave-3 refactor artifact.

module/receiver/ncclfrreceiver/nccl_fr_test.go (logsSink.first)
✅ Verified zero-caller. Drift cleanup post-PR #435 (promotion to consumertest.LogsSink) is appropriate.

scripts/validator-recipe.sh (EXAMPLES_DIR)
✅ Verified SC2034 (unused variable) via shellcheck.

Blocking Issue

PR body states: "Filed-as-followup rather than bundled here to keep this PR reviewable."

Before merge: File a tracking issue for the ~199 pre-existing module/ lint findings (errorlint, exhaustive, forcetypeassert, goconst, gocyclo, gosec, perfsprint, prealloc, predeclared, revive, staticcheck, testifylint, wrapcheck) and link it in the PR body. Per repo guidelines, load-bearing deferrals require an issue number before merge.

Suggested Action

Trim Makefile comment to 4–5 lines (workspace-mode root cause + --no-config rationale only).
File gh issue create for 199-findings module/ cleanup; link via fixes #NNN in PR body.
Merge.

Grade: A (minor comment bloat; one blocking issue link)**

trilamsr · 2026-06-02T05:20:13Z

Follow-up filed for ~199 deferred lint findings: #490 (expand module/ coverage from 'unused' to full .golangci.yml). Nits 1+2 minor — main Makefile comment trim deferred to that follow-up's PR. Arming auto-merge.

#498) ## Summary Triple-shipper closing three load-bearing infra debts that recurred on every chart/CI-touching PR. Atomic so we handle this once. ### Part 1 — Makefile sharding (cascade-rebase tripwire) **Root cause:** The root `Makefile` carried four monolithic prereq lists (`.PHONY:`, `check:`, `verify:`, `ci-fast:`, `ci-full:`). Every new gate appended one token to each list, and two open PRs touching the same line produced a 3-way merge conflict that required manual fix-up — the dominant source of cascade-rebases on this repo. **Fix:** Split into `make/{phony,check,verify,ci-fast,ci-full}.mk` shards using `+=` appends. Main `Makefile` `include`s the shards; aggregate targets now consume `$(*_DEPS)`. Prereq sets are logically equivalent to `origin/main` (modulo intentional gate additions): `make -pn` shows `lint-unused-module` replaced by `lint-module-full` in `check` (Part 3) and the new `makefile-hotfile-check` added to `ci-fast`/`ci-full` (Part 1 A+). No other prereq tokens moved. **A+:** Added `scripts/makefile-hotfile-check.sh` (+ `make makefile-hotfile-check` target) that fails if a future PR re-inlines prereq tokens into the root `Makefile`. Wired into `ci-fast` + `ci-full` so drift trips per-PR. ### Part 2 — Kind-CRD bootstrap composite action **Root cause:** Three workflows (chart.yml, policy-matrix.yml, install-bench.yml) each separately installed helm + kind + the tracecore image, each drifted from the others on CRD prereqs (ServiceMonitor #494 fixed policy-matrix only; chart.yml + install-bench.yml remained vulnerable to the same regression). **Fix:** Created `.github/actions/kind-cluster-setup/action.yml` as a single source of truth: pinned helm v3.16.4, kind v0.25.0, node v1.32.0, ServiceMonitor CRD v0.91.0 (#494 pin), with toggles for Gatekeeper / cert-manager CRDs (reserved for future workflows). All 3 workflows now `uses:` the composite. Old `kind-tracecore-up` shim deleted (zero remaining callsites). **Mutation-verify:** changing the ServiceMonitor CRD URL in `kind-cluster-setup/action.yml` fails all 3 workflows uniformly by construction (single-source-of-truth pin). ### Part 3 — Full module/ lint coverage (#490 follow-up of #486) **Root cause:** `make lint` from the root never reaches `module/` (workspace mode resolves `./...` only inside the current module). PR #486 added `make lint-unused-module` for the `unused` linter only; the rest of the 13 linters declared in `.golangci.yml` were silently skipped against `module/`, accumulating 57 findings. **Fix:** New `make lint-module-full` target runs the full `.golangci.yml` linter set against `module/`. Swept all 57 pre-existing findings to 0: - `golangci-lint --fix`: 17 findings auto-fixed (testifylint 14, errorlint 1, perfsprint 1, staticcheck-QF1003 1) - Real fixes: forcetypeassert → checked assertions (6); goconst → `fallbackCollectiveOp`, `PressureUnknown` constants (4); gocritic rewrites (2); predeclared renames (3); revive package-comments (1); prealloc (1); wrapcheck `fmt.Errorf "%w"` (2); G301 mkdir 0755 → 0750 (1); exhaustive → explicit `default:` clauses (7) + explicit `PressureUnknown` case (1) - Documented opt-outs with per-finding rationale: gocyclo on 5 pattern-match dispatch funcs whose complexity tracks vocabulary cardinality, not nested logic; gosec G103 on audited `unsafe.String` aliasing carve; gosec G304 on 5 test-local fixture reads; staticcheck SA1019 on explicit pre-deprecation parity assertion (#277) `make lint-module-full` exits 0 on the cleaned tree. Wired into `make check`, replacing `lint-unused-module` (retained for fast-iteration dead-code sweeps). ## Per-part metrics | Part | Metric | Before | After | |---|---|---|---| | 1 | Makefile aggregate-list LoC (hot lines) | 5 monolithic lines | 5 `include` lines | | 1 | Make-target prereq sets vs `origin/main` | — | logically equivalent (modulo intentional gate additions; see Part 1 Fix) | | 1 | New shards under `make/` | 0 | 5 (`phony`, `check`, `verify`, `ci-fast`, `ci-full`) | | 2 | Composite action wired into N workflows | 0 (each duplicated kind setup) | 3 (chart, policy-matrix, install-bench) | | 2 | CRDs covered | 0 (ad-hoc per workflow) | ServiceMonitor (now), Gatekeeper + cert-manager (reserved) | | 2 | Workflow LoC delta (kind setup blocks) | 3× ~10 lines duplicated | 3× ~10 lines (composite-action `uses:` blocks) | | 3 | Lint findings (module/) | 57 (across 14 linters) | 0 | | 3 | Linters enabled against module/ | 1 (`unused`) | 13 (full `.golangci.yml` set) | | 3 | Documented opt-outs | — | 14 `//nolint:` directives, each with per-finding rationale | ## Closes - #490 (full module/ lint coverage) - Reaffirms #486 (extends from `unused` only to the 13-linter set) ## Follow-ups filed - #497 — surfaced (not caused) by this PR: `TestPatternDetector_NegativeFixturesEmitNoVerdicts/synthetic-2026-06-multi-rank-disk-pressure` fails on `origin/main` HEAD because the negative-fixture filter treats every non-canonical fixture as negative, including the `_real_world/*` positives added in #484. Fix sketch + repro included. ## Test plan - [x] `make verify` — green - [x] `make check` — green - [x] `make lint-module-full` — exit 0 - [x] `make doc-check` — green (comment-noise sweep) - [x] `make actionlint` — green - [x] `make zizmor` — green - [x] `make makefile-hotfile-check` — green (own gate) - [x] `make -pn` byte-identity check against `origin/main` for all 4 aggregate targets — passes (modulo intentional `makefile-hotfile-check` additions to ci-fast/ci-full + `lint-module-full` superseding `lint-unused-module` in check) - [x] `(cd module && GOWORK=off go test ./...)` — green except pre-existing #497 failure (filed) - [ ] CI green on this PR (kind workflows actually exercise the new composite action) ```release-notes infra: shard Makefile into make/*.mk (cascade-rebase prevention per [[rebase-cascade]]); unify 3 workflows behind .github/actions/kind-cluster-setup composite (CRD prereq install + pinned tools); enable full module/ lint coverage (57 findings → 0, 13 → 27 linters wired into make check). Closes #490; refs #494/#496. ``` --------- Signed-off-by: Tri Lam <tree@lumalabs.ai>

trilamsr mentioned this pull request Jun 2, 2026

ci(lint): expand module/ coverage from 'unused' to full .golangci.yml #490

Closed

trilamsr merged commit 6edb19d into main Jun 2, 2026
27 checks passed

trilamsr deleted the chore/dead-code-sweep-2026-06 branch June 2, 2026 05:20

trilamsr mentioned this pull request Jun 2, 2026

chore(infra): Makefile shards + kind-CRD bootstrap + module/ full-lint #498

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore: sweep dead code + close module/ lint-gate gap#486

chore: sweep dead code + close module/ lint-gate gap#486
trilamsr merged 1 commit into
mainfrom
chore/dead-code-sweep-2026-06

trilamsr commented Jun 2, 2026

Uh oh!

trilamsr commented Jun 2, 2026

Uh oh!

trilamsr commented Jun 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

trilamsr commented Jun 2, 2026

Summary

Root cause (the lint-gate gap)

Deletions

CI gate (the fix, not just a workaround)

Validation that the gate would have caught this

Out of scope (filed as follow-up)

Test plan

Uh oh!

trilamsr commented Jun 2, 2026

Independent Adversarial Review: PR #486

Success Criteria Audit

Findings

Blocking Issue

Suggested Action

Uh oh!

trilamsr commented Jun 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant