Fix docs-review smart-gate bugs and reduce PR comment spam#1536
Open
justinegeffen wants to merge 3 commits into
Open
Fix docs-review smart-gate bugs and reduce PR comment spam#1536justinegeffen wants to merge 3 commits into
justinegeffen wants to merge 3 commits into
Conversation
Three independent fixes that all affect the /editorial-review path:
1. Smart-gate base_ref resolution
github.base_ref and github.head_ref are empty on issue_comment events,
so the previous diff `origin/${{ github.base_ref }}...HEAD` expanded to
`origin/...HEAD` and the smart-gate's checkout step pulled the default
branch rather than the PR head. Net effect: lines_changed was 0 on the
comment trigger and the <10-lines skip always fired. Resolve both refs
via `gh pr view --json baseRefName,headRefName` (matching what the
`classify` job already does) and check out the PR head before diffing.
2. Smart-gate static_issues counter
`grep -c ":" /tmp/static-results.txt` counted every colon in
markdownlint output, not actual lint issues. Markdownlint output has
multiple colons per issue line and also prints success lines. The "fix
formatting first" gate was tripping (or not) for the wrong reasons.
Count lines matching `MD[0-9]{3}` instead.
3. PR comment spam reduction
A single /editorial-review previously produced up to four PR comments:
the "started" acknowledgement, a sticky comment with Claude's progress,
the consolidated review with inline suggestions, and a summary table.
The sticky comment duplicates information already visible on the
workflow run UI, and the summary table duplicates information already
on the consolidated review. Drop both. End result: 1 acknowledgement +
1 review per /editorial-review.
Vale extraction (PR #1535) removes the vale-lint job from this workflow;
the summary job that referenced it is also removed here so this PR is
mergeable independently.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for seqera-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Signed-off-by: Justine Geffen <justinegeffen@users.noreply.github.com>
Signed-off-by: Justine Geffen <justinegeffen@users.noreply.github.com>
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
Three independent fixes on the
/editorial-reviewpath in docs-review.yml. All affect theissue_commenttrigger, which is the primary way the workflow runs.1. Smart-gate base_ref resolution
github.base_refandgithub.head_refare empty onissue_commentevents. The previous difforigin/\${{ github.base_ref }}...HEADexpanded to `origin/...HEAD`, and the smart-gate'scheckoutstep (without aref:) pulled the default branch instead of the PR head. Net effect:lines_changedwas effectively 0 on every comment trigger and the <10 lines changed skip always fired — so the LLM review was being blocked for the wrong reason.Fix: resolve both refs via `gh pr view --json baseRefName,headRefName` (matching what the
classifyjob already does) and check out the PR head before diffing.2. Smart-gate static_issues counter
`grep -c ":" /tmp/static-results.txt` counted every colon in markdownlint output, not lint issues. Markdownlint prints `path:line:col` (3 colons per issue line) plus success lines that also have colons in paths, so the >5 issues = skip gate tripped (or not) based on noise. Replace with `grep -cE 'MD[0-9]{3}'` to count lines that actually carry a rule code.
3. PR comment spam reduction
A single `/editorial-review` previously produced up to four PR comments:
claude-code-actionshowing progressThe sticky comment duplicates the workflow-run UI linked from #1. The summary table duplicates the inline review in #3. Removed #2 (`use_sticky_comment: false`) and #4 (deleted the `summary` job entirely). End state: 1 ack + 1 review per run.
Coordination with PR #1535
PR #1535 extracts Vale to its own workflow and removes `vale-lint` from this file. The summary job referenced `vale-lint` directly; this PR independently deletes the summary job. Merging either order is fine — there's no semantic conflict, only a textual one git can auto-resolve. Recommend merging #1535 first.
Test plan
.mdchanges. Confirm smart-gate logs show non-zerolines_changedand the LLM review runs.static_issuescount matches the actual issue count from `markdownlint-cli2` locally.🤖 Generated with Claude Code