ci: skip doc-check when no changed path has a documentation surface - #29364
Merged
nickvigilante merged 1 commit intoSep 15, 2026
Conversation
doc-check started a review chat on every non-draft pull request. A review loads the full skill context, the content guidelines, and the cumulative diff before it can conclude anything, so a dependency bump or a test-only change paid that cost only for the skill's own "what not to comment on" list to produce silence. With the wait now at 40 minutes, such a run can also hold a runner idle for most of that. Classify the changed paths first and skip the review when every changed file is in a class that has no user-facing documentation surface: tests and stories, generated output, dependency manifests, build and lint plumbing, styling, internal environments, and vendored trees. Comparing the match count against the total is what makes this "every file", so a diff that mixes a test file with a CLI flag still gets a full review. CI that builds, deploys, previews, or reviews the docs is carved back out of the .github skip, because a change there can change the docs themselves. The doc-check label and a manual dispatch bypass the skip, so a review stays forceable from the pull request. Record the tier-2 and tier-3 priors in path-priors.md, loaded only when the pre-filter is inconclusive, and point the skill at it. DOCS-920
bpmct
approved these changes
Sep 15, 2026
nickvigilante
deleted the
vigilante/docs-920-doc-check-add-a-ci-path-pre-filter-and-three-tier-path
branch
September 15, 2026 21:14
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
doc-check started a review chat on every non-draft pull request. A review loads the full skill context, the content guidelines, and the cumulative diff before it can conclude anything, so a dependency bump or a test-only change paid that cost only for the skill's own "what not to comment on" list to produce silence. With the wait raised to 40 minutes in #29363, such a run can also hold a runner idle for most of that.
This classifies the changed paths first and skips the review when every changed file is in a class with no user-facing documentation surface. Comparing the match count against the total is what makes it "every file", so a diff that mixes a test file with a CLI flag still gets a full review.
CI that builds, deploys, previews, or reviews the docs is carved back out of the
.githubskip, because a change there can change the docs themselves. Thedoc-checklabel and a manual dispatch bypass the skip entirely, so a review stays forceable from the pull request.Tier-2 and tier-3 priors move into
path-priors.md, loaded only when the pre-filter is inconclusive, so they cost nothing on a skipped run.Verification
Dry-ran the filter list against real and synthetic file lists with the same globber the action uses.
ci_docscarve-out firesgo.mod+go.sumsite/src/index.csscli/server.go+ its testcli/list.go.github/workflows/release.yamlmake pre-commitpasses, includinglint/actions/actionlint.Why the patterns use the
**/*formThe globber treats a
**glued to a suffix inconsistently. Verified against picomatch withdot: true:coderd/database/db_test.gofoo_test.go**_test.go**/*_test.go**.test.tshappens to match at any depth, but**_test.goonly matches the repo root, so a nested Go test would have leaked through and started a review. Every pattern in the filter uses**/*instead, which behaves the same at every depth including the root..github/workflows/ci.yamlcarries a related note about the same class of bug in a different action's globber.Implementation plan and decision log
Part of a broader restructure of the doc-check skill. The audit found five defects: no path pre-filter, no commit scoping, duplicated rules, no likelihood model, and no destination for adjacent docs-gap ideas. This PR addresses the first, which is the largest token win and the lowest risk. The remainder is tracked separately.
Rejected:
paths-ignoreon thepull_requesttrigger. It does express "all changed files match" for free, which was the initial plan. But it applies to the trigger as a whole, includinglabeledandready_for_review, so adoc-checklabel on a CSS-only pull request would have been filtered out too and the manual override would have silently died. A job-level filter step costs about 20 seconds of runner time and keeps the override working, so the filter lives in the job and the trigger is untouched.Rejected: negation patterns for the docs-affecting CI carve-out.
!-prefixed patterns inside thenodocsfilter would have made the result depend on pattern order. A separateci_docsfilter that must be empty is explicit and reads as what it means.Deliberately not skipped:
coderd/database/migrations/. A migration alone has no user surface, but migrations usually ship next to an API change, and the all-files-must-match rule already handles that case. Left to the agent rather than hardcoded either way.Known limitation. A user-facing default change can hide inside an otherwise-skippable path. The all-files-must-match rule plus the label override is the mitigation; a path-only gate cannot close this completely.
Follow-ups this does not do. Commit-scoped incremental reviews, a
concurrencyblock, splittingSKILL.mdand deleting the rules it restates from the content guidelines, run-outcome reporting so silence and failure are distinguishable, and routing adjacent docs-gap ideas into Linear.DOCS-920