Skip to content

chore(hooks): dedup gate execution — split into fast/medium/CI tiers#172

Merged
trilamsr merged 3 commits into
mainfrom
chore/dedup-gate-execution
May 31, 2026
Merged

chore(hooks): dedup gate execution — split into fast/medium/CI tiers#172
trilamsr merged 3 commits into
mainfrom
chore/dedup-gate-execution

Conversation

@trilamsr

@trilamsr trilamsr commented May 31, 2026

Copy link
Copy Markdown
Contributor

What this PR does

Eliminates triple-counted gate execution in the local hooks. Per push, 21 distinct gates ran an aggregate of 40 times (48% wasted overhead):

  • 2 gates ran 3× (lint, tidy-check) — pre-commit + pre-push + GH Actions
  • 15 gates ran 2× — pre-push + GH Actions
  • 4 gates ran 1× — correct

Fix

Split gates into three cost tiers, each executed exactly once:

Tier Gates Where Time
Fast fmt, tidy-check, lint, vet, mod-verify pre-commit (make check-fast) ~6s
Medium check-fast + license-check, generate-check, generate-fixtures-check, build-tags, nccl-fr-rce-gate, register-lint, actionlint, zizmor, doc-check, no-autoupdate-check pre-push (make check-medium) ~25s
Slow test, coverage-check, govulncheck, ci-fuzz-nccl-fr, build, validator-recipe GitHub Actions only (CI)

make check and make ci are retained unchanged for manual / contributor use; only the two hooks point at the new lean targets.

Root cause

Pre-commit was make check (includes test), pre-push was make ci (superset of check — re-runs lint + tidy-check + adds test, govulncheck, coverage, fuzz, build). GH Actions then runs the same gates remotely. No deduplication between layers.

Effect

  • Local wall time per push: ~3 min → ~31s (~80% reduction)
  • CI gate coverage: unchanged
  • Lost local fast-fail signal: test, govulncheck, coverage, fuzz, build. These still gate merge via required CI checks.

Release notes

NONE

Test plan

  • make check-fast runs and passes (6.5s observed)
  • make check-medium runs and passes (25s observed)
  • Pre-commit hook fires make check-fast on commit (this PR's own commit)
  • Pre-push hook fires make check-medium on push (this PR's own push)
  • make check and make ci definitions unchanged

Before this change, gate execution was triple-counted: every `git push`
ran 21 distinct gates an aggregate of 40 times (48% overhead). Lint and
tidy-check ran 3× (pre-commit + pre-push + GitHub Actions); 15 gates
ran 2×; only 4 ran once.

The fix splits gates into three tiers by cost, each run exactly once:

- `make check-fast` (new, ~6s): fmt, tidy-check, lint, vet, mod-verify.
  Pre-commit gate. Catches typos, format drift, dep drift.
- `make check-medium` (new, ~25s): check-fast + license-check,
  generate-check, generate-fixtures-check, build-tags, nccl-fr-rce-gate,
  register-lint, actionlint, zizmor, doc-check, no-autoupdate-check.
  Pre-push gate. Adds machine-validatable static analysis.
- GitHub Actions only: test, coverage-check, govulncheck, ci-fuzz-nccl-fr,
  build, validator-recipe. Slow gates (>15s each) deferred to CI to
  keep `git push` fast; CI will catch failures before merge.

`make check` and `make ci` retained unchanged for manual / contributor use.

Net effect per push: ~31s local (was ~3 min) → ~80% local-wall-time
reduction. CI gate coverage unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Signed-off-by: Tri Lam <tri@maydow.com>
@trilamsr trilamsr enabled auto-merge (squash) May 31, 2026 00:16
Tri Lam added 2 commits May 30, 2026 17:26
Move composition details to Makefile docstrings where they live anyway.
Hook header keeps only the bypass instruction.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Signed-off-by: Tri Lam <tri@maydow.com>
@trilamsr trilamsr merged commit cc0ff3f into main May 31, 2026
10 checks passed
@trilamsr trilamsr deleted the chore/dedup-gate-execution branch May 31, 2026 00:32
trilamsr added a commit that referenced this pull request May 31, 2026
## What this PR does

Three small cleanups that consolidate the gate UX after PR #172 landed
the dedup split:

### 1. Rename gate tiers for clarity

| Old | New | Where | Time |
|---|---|---|---|
| `check-fast` (PR #172) | `check` | pre-commit | ~6s |
| `check-medium` (PR #172) | `verify` | pre-push | ~25s |
| Old `check` (had `test`) | **deleted** | — | — |
| `ci` | `ci` (unchanged) | manual / GitHub Actions | full |

PR #172 introduced `check-fast` + `check-medium` to dedup gate
execution, but the names leaked the implementation detail (tier).
Renaming to `check` / `verify` matches what they actually do — quick
self-check vs verify-before-push. The old `check` (which included
`test`) is removed; `make test` is still a target if you want it
standalone, but the slow test gate now runs only in CI.

Hooks (`.githooks/pre-commit`, `.githooks/pre-push`) point at the new
names; `make hooks` echo block updated.

### 2. Split the .PHONY line into 8 logical groups

The Makefile had a single 60-target `.PHONY` line, ~1.2KB wide. Split
into:
- build, test, format, generate, coverage, policy gates, aggregate
gates, release+integration

Zero semantic change. Readability win for anyone editing the Makefile.

### 3. Add pr-lint workflow step that rejects shell-heredoc body
artifacts

PRs #171 and #172 both shipped with literal backslash-backtick artifacts
in their descriptions because the bodies were composed inline via `gh pr
create --body "$(cat <<'EOF' ... EOF)"` and over-escaped backticks.
Renders as visible `\` on GitHub.

The new pr-lint step counts occurrences and fails when 3+ are present
(plural is the artifact signature; a single mention in prose describing
the bug is fine — this PR body itself contains one). Error message
points to `gh pr edit --body-file FILE.md` as the working pattern.

## Root cause

- **Gate names:** tier was the wrong abstraction for the UX-facing name.
Speed-tier maps 1:1 to which hook fires it, so the hook stage (`check` =
before-commit, `verify` = before-push) is what users actually think in.
- **PR body artifact:** shell heredoc `<<'EOF'` (quoted) preserves
literals correctly. Adding `\\\`` thinking shell would expand the
backtick produced literal backslash + backtick. The workflow now blocks
this class of artifact at lint time, scoped to 3+ occurrences so prose
mentioning the pattern doesn't false-positive.

## Release notes

```release-notes
NONE
```

## Test plan

- [x] `make check` runs and passes (~6s observed)
- [x] `make verify` runs and passes (~25s observed)
- [x] Pre-commit hook fires `make check` on commit
- [x] Pre-push hook fires `make verify` on push
- [x] `make hooks` echo block reflects new names
- [x] `make actionlint` passes (new pr-lint step is YAML-valid)
- [x] New rule does NOT fire on this PR body (1 mention, threshold is 3)

---------

Signed-off-by: Tri Lam <tri@maydow.com>
Co-authored-by: Tri Lam <tri@maydow.com>
trilamsr added a commit that referenced this pull request May 31, 2026
## What this PR does

Bundles three RFC-0013 PR slices that have zero file overlap with each
other.

### PR-C: release pipeline → goreleaser stack

- New `.goreleaser.yaml`: linux/amd64 + linux/arm64 builds; reproducible
via `SOURCE_DATE_EPOCH`; LDFLAGS shape matches the Makefile build
target.
- Rewritten `.github/workflows/release.yml`: invokes goreleaser,
`anchore/sbom-action`, `sigstore/cosign-installer`,
`slsa-framework/slsa-github-generator` (tag-pinned per SLSA OIDC subject
identity requirement; all other actions SHA-pinned per repo security
policy), `actions/attest-build-provenance`.
- Old `release.yml` moved to
`.github/workflows/archived/release.yml.legacy`.
- Goreleaser builds the **legacy** `cmd/tracecore` binary; OCB-output
migration deferred to PR-D (image build → ko), per inline comment in
`.goreleaser.yaml`.

### PR-G + PR-H: RFC supersession + top-level docs alignment

- Audit confirmed all 12 RFCs already carry the correct supersedence
headers from prior pivot work (PRs #166/#168/#169/#170). Only two
top-level docs needed alignment:
- `NORTHSTARS.md` O1 caveat: replaced "own-binary architecture"
assumption wording with OCB-distribution-posture wording; closed Open
Question #1 by RFC-0013 ref.
- `CHANGELOG.md`: appended pivot-wave-1 PR list
(#166/#168/#169/#170/#171/#172/#173) citing PR-A as the prior step
before this commit.
- No edits needed to
README/STRATEGY/PRINCIPLES/MILESTONES/CONTRIBUTING/AGENTS/docs/README —
all already aligned.

### PR-E: clockreceiver swap — BLOCKED

- `telemetrygeneratorreceiver` does not exist in
`opentelemetry-collector-contrib` at any version. Verified against the
Go module proxy, GitHub tree API at v0.95→v0.130, and the full receiver
listing at v0.110.0 (94 receivers; no `telemetrygenerator`, `loadgen`,
`mockreceiver`, `dummyreceiver`, or any `*generator*`). The RFC-0013 §1
example shape referenced it speculatively; it was never upstreamed.
- `builder-config.yaml`: replaced the misleading "no v0.110.0 tag"
omission comment with a verified TODO block describing the actual
blocker (receiver doesn't exist anywhere) and decision rationale.
- `bench/install/tracecore-values.yaml`: appended `[BLOCKED]` marker on
the clockreceiver→telgen mapping; bench continues to use in-tree
clockreceiver until PR-F deletes it (likely rewires to
`hostmetricsreceiver`).

## Root cause (PR-E blocker)

RFC-0013 §1 listed `telemetrygeneratorreceiver` as the swap target
without verifying the receiver existed upstream. Reality: the OTel
contrib repo has no such module path at any tag. PR-E cannot complete
until either (a) the receiver lands upstream, or (b) a different
replacement is chosen (e.g., `hostmetricsreceiver` for heartbeat
semantics). Tracked in the in-file TODO block; revisit in PR-F (delete
clockreceiver) or as a separate followup.

## Release notes

```release-notes
[CHANGE] Release pipeline migrated to goreleaser + SBOM + SLSA provenance + cosign signing. The release.yml workflow now invokes goreleaser instead of building binaries directly. Operators consuming release artifacts: artifact shape (filename, archive contents, checksum file format) follows goreleaser defaults; see CHANGELOG.md for the migration note.
```

## Test plan

- [x] `make verify` runs and passes
- [x] `make actionlint` passes (new release.yml workflow +
suppression-block YAML valid)
- [x] `make zizmor` passes (SLSA reusable-workflow tag-pin justified
inline + accepted)
- [x] `make build` (legacy) still works
- [x] `make build-ocb` (OCB) still works
- [ ] Goreleaser dry-run in CI on first push to a tag (gated until a tag
exists)

Signed-off-by: Tri Lam <tri@maydow.com>
Co-authored-by: Tri Lam <tri@maydow.com>
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.

1 participant