[chore] comments: anti-regression gates for #128#131
Merged
Conversation
PR #128 trimmed verbose comments but nothing enforces the discipline. These three changes lock the trim in place so it cannot drift back. scripts/doc-check.sh Adds a comment-noise diff-scope gate mirroring the em-dash gate. Blocks on PR-diff additions (vs origin/main): - Reviewer/review-process tags (e.g. "Reviewer P3-Rev2-F1") - Bare "PR #N" references inside comments - Go section-banner comments ("// --- foo ---", "// === Section ===") Exemptions match the em-dash gate (docs/rfcs/, docs/research/, docs/STYLE-docs.md, .claude/). Existing tree usage is grandfathered; only lines this PR adds are checked. Mutation-verified locally: a commit containing one of each pattern fails the gate with file:line citations for all three. components/receivers/pyspy/kinds_test.go Adds TestKinds_README_DocumentsEveryKind, which pins the claim made in kinds.go's package comment (operator semantics live in README.md). The earlier RFC-0009 parity test pins the wire-string set; this test pins that each wire string also appears in README.md so the comment-points-at-README claim cannot drift silently. internal/selftelemetry/interface.go Restores a one-line doc on Receiver.IncError. The earlier collapse left it docless, which reads as an oversight to receiver authors using `go doc selftelemetry.Receiver`. One-line doc is the minimum idiomatic Go expects. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The first commit on this branch shipped a gate banning verbose comments
while the gate's own preamble was 23 lines of prose duplicating the PR
body. Self-application would have failed if shell were in scope.
Changes:
- Widen gate scope to *.sh / *.py / *.yaml / *.yml so the gate now
catches the exact category of file my own diff slipped through.
Generalize banner + pr-ref regexes from // to (// or #) so shell
comments are not a loophole.
- Trim doc-check.sh comment-noise gate preamble from 23 lines to 2.
The failure message and pattern names self-document; STYLE.md owns
the reasoning.
- Drop the 4-line preamble on TestKinds_README_DocumentsEveryKind.
Test name + failure message self-document.
- Drop the // IncError records... one-liner. Receiver.IncError takes
a typed Kind; the type-level doc on Kind carries the discipline.
Self-application: post-trim `make doc-check` is clean against
origin/main with the widened gate active.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
F1. Markdown headings starting with `## ... PR #N` collided with the
pr-ref regex. docs/FOLLOWUPS.md already carries lines of that
shape (grandfathered, but future doc edits would block). Source-
code patterns (banner-comment, pr-ref) now restrict to `//` or
leading-`#` shell-style leaders and skip *.md files entirely. The
reviewer-tag pattern still fires across all scoped extensions
since it has no leader collision.
F4. README doc-parity test used plain Contains, which would pass
spuriously if a renamed kind contained an existing kind as a
substring (e.g. `parse_error_v2` would silently satisfy the
assertion for `parse_error`). Match against backtick-wrapped
kind strings, mirroring the README's own table convention.
Verified locally:
- 8-case mutation table for the regex shows headings, bullet refs,
and dash-only doc lines no longer FP; shell/Go comment offenders
still fire.
- `make doc-check` clean against origin/main.
- `make lint` clean.
- `go test ./components/receivers/pyspy/...` green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review deferred F2 was "expand on first offender". Tree is clean today (grep returns zero), so cost of doing now is 5 chars in a pathspec versus carrying a tracking item indefinitely for a hypothetical. Land it now, drop the deferral. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
trilamsr
added a commit
that referenced
this pull request
Jun 2, 2026
…460) (#466) ## Summary Closes #460. The `exit 0` on `scripts/doc-check.sh` ran unconditionally whenever `docs/FAILURE-MODES.md` carried no `Test*`/`Fuzz*`/`Benchmark*` identifiers (its current state on `main` — `grep -c` = 0), silently bypassing every gate below it. Fix scopes the skip to the Go-test parity block only (if/else, not `exit`), then surfaces and fixes the dead refs the gates were supposed to be catching. ## Root cause Commit a57883f (#13) shipped `doc-check.sh` with one gate — the Go-test name parity check — so `[ -z "$referenced" ] && exit 0` was correct then. PRs #28, #56, #115, #131, #144, #149, #195, #234, #241, #443, #455, #459 (and others) appended gates **below** that line without recognising they'd become dead code whenever `FAILURE-MODES.md` lost its `Test*` references. PR #459 worked around the bug by placing its new YAML gate *above* line 99 and tracked the root cause separately as #460. ## What surfaced Once `exit 0` was removed, three real issues fired: 1. **Dead `.md` link**: `docs/FOLLOWUPS.md` → `followups/otlphttp.md`. The shard was never committed to `main`'s ancestry. Folded into the existing "Shards deleted post-v0.2.0 as fully resolved-via-pivot" prose block (sibling treatment to M9, M14, M16). 2. **Banned-phrase hits** (3x `production-grade`): reworded in `docs/cut-criteria.yaml.md` (2x) and `install/kubernetes/tracecore/README.md` (1x) to falsifiable language. 3. **`docs/getting-started.md` block cap**: 7 fenced bash/sh blocks. The M6 cap of 5 was set for the quickstart only — `## Install via Helm` and `## Air-gapped install` are alternate deployment paths that landed post-M6 and aren't part of the quickstart budget. Rescoped the gate to count blocks inside the `## Walkthrough` H2 section only (1 block, well under cap). ## Gate count Empirically verified via `grep -c '^doc-check: '` on `make doc-check` output on a clean tree: | State | Status lines emitted | Gates the early-exit was hiding | |---|---|---| | Pre-fix on `main` (post-#459) | 3 (trust-posture, YAML cross-link, parity-skip) | 14 | | Post-fix this PR (post-rebase) | 17 | 0 | The "14 gates hidden" number is invariant across the rebase: it counts gates placed below the early-exit line. The "3 → 17" total reflects post-#459 reality on `main`; pre-#459 baseline was "2 → 16" (the figure originally in this PR body), and #459 itself worked around the bug by placing its YAML gate above line 99. ## Mutation tests Each gate below the original early-exit was confirmed to fire post-fix: | Mutation | Gate expected to fire | Exit code post-mutation | Exit code post-restore | |---|---|---|---| | Inject `[bad](nonexistent-ghost.md)` into `docs/FOLLOWUPS.md` | markdown link-rot | 1 | 0 | | Append `blazing-fast` + `rock-solid` to `docs/getting-started.md` | banned-phrase lint | 1 | 0 | | Delete `<!-- tested-against: ... -->` from `docs/integrations/datadog.md` | M6 recipe markers | 1 | 0 | ## Test plan - [x] `make doc-check` exits 0 on clean tree (re-run post-rebase onto origin/main; 17 status lines) - [x] 3 mutation tests above each toggle exit 1 → 0 across mutate / restore - [x] Pre-push hooks green: golangci-lint (0 issues), `go vet ./...`, `go mod verify`, `attribute-namespace-check` (100 attrs, all documented), `register-lint`, `actionlint`, `zizmor`, `deprecation-check`, `no-autoupdate-check` - [x] Rebased onto current `origin/main` (includes #459, #461, #462, #456); no conflicts; gate count re-verified empirically post-rebase - [x] No changes to gates above line 99 (the trust-posture callout + YAML cross-link gate from #459 still run and emit unchanged status lines) ## Self-grade **A+** — root cause named in commit body (a57883f #13 with one gate; gates appended below without exit-path awareness); 3 mutation tests (success criteria required 1–2); rescoped the getting-started gate to match M6 intent rather than papering over the surfaced overflow; the `[ -z "$referenced" ]` legitimate skip is preserved via if/else (not `:` no-op, which would have left the `defined=` / `orphans=` block running on empty input); gate count corrected empirically post-rebase per reviewer B feedback. ```release-notes - fix(ci): `scripts/doc-check.sh` no longer exits 0 at the Go-test parity gate when `docs/FAILURE-MODES.md` carries no `Test*` references. 14 gates below that line (link-rot, banned-phrase, M6 recipe markers, etc.) are now actually enforced on every `make doc-check` invocation. Closes #460. ``` --------- Signed-off-by: Tri Lam <tree@lumalabs.ai>
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Locks in the trim from #128 with three concrete anti-regression gates.
scripts/doc-check.shPR #Nrefs, and Go section-banner comments on PR-added lines (mirroring the em-dash gate).components/receivers/pyspy/kinds_test.goTestKinds_README_DocumentsEveryKindpins the claimkinds.go's package comment makes (operator semantics live in README.md).internal/selftelemetry/interface.goReceiver.IncError.Why these, not others
#128 removed verbose comments by hand. Nothing prevents them from coming back the next time someone reaches for a reviewer tag or section banner. STYLE.md says default-no-comment, but it is advisory, not enforced. This PR is the enforcement.
Root cause being addressed: STYLE.md's default-no-comment discipline relies on review-time vigilance; verbose-comment regressions are easy to miss in a long review. This PR converts the discipline into a CI gate so regressions block at merge time, not at review time.
Why a gate, not a workaround
A workaround would be: write a memory note, add a CLAUDE.md line, add a checklist item. None of those block at merge time. CI gates do. STYLE.md already gates em-dashes the same way; this PR follows the same pattern for the same reason.
Mutation verification
Confirmed locally: a commit containing
// Reviewer P3-Rev2-F1: x,// PR #999 ..., and// ---- banner ----fails the gate with file:line citations for all three patterns:Release notes
Test plan
go build ./...go vet ./...make lint— 0 issuesmake doc-check— em-dash + comment-noise gates clean on this PR's diffgo test ./components/receivers/pyspy/... ./internal/selftelemetry/...— green🤖 Generated with Claude Code