From 76318d4768f7b82daeba41373dc0b461e187c24c Mon Sep 17 00:00:00 2001 From: jth-nw Date: Fri, 1 May 2026 12:46:39 -0500 Subject: [PATCH 1/3] fix(doc-pr): add @claude acknowledgment comment and skip re-review after fixer commits --- .claude/skills/doc-pr-fix/SKILL.md | 16 +++++++++++++--- .github/workflows/claude-doc-pr.yml | 17 +++++++++++++++-- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/.claude/skills/doc-pr-fix/SKILL.md b/.claude/skills/doc-pr-fix/SKILL.md index 99120834e3..5f8c22a2be 100644 --- a/.claude/skills/doc-pr-fix/SKILL.md +++ b/.claude/skills/doc-pr-fix/SKILL.md @@ -47,17 +47,27 @@ Example tasks for a "fix all issues" request: Only include tasks for what the writer actually asked for. The task list must reflect the writer's request exactly. -Then post a PR comment mirroring your task list so the writer can see what you're doing: +Then update the acknowledgment comment (already posted by the workflow) with your task list. The comment ID is in `$PROGRESS_COMMENT_ID`: ```bash -PROGRESS_COMMENT_ID=$(gh pr comment "$PR_NUMBER" --body "$(cat <<'EOF' +gh api repos/{owner}/{repo}/issues/comments/$PROGRESS_COMMENT_ID \ + -X PATCH -f body="$(cat <<'EOF' **Fix in progress:** - [ ] Apply editorial suggestions in `path/to/file.md` - [ ] Verify changes - [ ] Commit and push EOF -)" --format json | jq -r '.id' 2>/dev/null || echo "") +)" +``` + +If `$PROGRESS_COMMENT_ID` is empty for any reason, fall back to creating a new comment: + +```bash +if [ -z "${PROGRESS_COMMENT_ID:-}" ]; then + PROGRESS_COMMENT_ID=$(gh pr comment "$PR_NUMBER" --body "**Fix in progress:** ..." \ + --format json | jq -r '.id' 2>/dev/null || echo "") +fi ``` As you complete each Todo task, also update the PR comment to check off the corresponding item: diff --git a/.github/workflows/claude-doc-pr.yml b/.github/workflows/claude-doc-pr.yml index 4ec4f2ad2b..0022620e73 100644 --- a/.github/workflows/claude-doc-pr.yml +++ b/.github/workflows/claude-doc-pr.yml @@ -35,8 +35,8 @@ jobs: run: | MESSAGE=$(gh api repos/${{ github.repository }}/commits/${{ github.event.pull_request.head.sha }} --jq '.commit.message') echo "Latest commit message: $MESSAGE" - if echo "$MESSAGE" | grep -qE '^fix\((vale|dale)\):|^ci: trigger build'; then - echo "Skipping: commit is from autofix workflow" + if echo "$MESSAGE" | grep -qE '^fix\((vale|dale)\):|^ci: trigger build|^docs: apply .* fixes from PR review'; then + echo "Skipping: commit is from autofix or doc-fixer workflow" echo "skip=true" >> "$GITHUB_OUTPUT" else echo "skip=false" >> "$GITHUB_OUTPUT" @@ -162,6 +162,18 @@ jobs: gh pr comment ${{ steps.pr-info.outputs.number }} --repo ${{ github.repository }} \ --body "This PR is from a fork. Automated fixes cannot be pushed directly. I can still review and suggest changes — apply them manually from the comments." + - name: Post acknowledgment comment + id: ack-comment + if: steps.pr-info.outputs.is_fork == 'false' && steps.pr-info.outputs.targets_dev == 'true' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + COMMENT_ID=$(gh pr comment "${{ steps.pr-info.outputs.number }}" \ + --repo "${{ github.repository }}" \ + --body "**Acknowledged:** Analyzing your request — I'll update this comment with a task list shortly." \ + --format json | jq -r '.id') + echo "comment_id=$COMMENT_ID" >> "$GITHUB_OUTPUT" + - name: Checkout repository if: steps.pr-info.outputs.is_fork == 'false' && steps.pr-info.outputs.targets_dev == 'true' uses: actions/checkout@v4 @@ -176,6 +188,7 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} COMMENT_BODY: ${{ github.event.comment.body }} + PROGRESS_COMMENT_ID: ${{ steps.ack-comment.outputs.comment_id }} with: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} github_token: ${{ secrets.VALE_TOKEN }} From 991ec52713613497111eb94fbe566aca7c4239cc Mon Sep 17 00:00:00 2001 From: jth-nw Date: Fri, 1 May 2026 12:52:20 -0500 Subject: [PATCH 2/3] fix(doc-pr): scope editorial review to diff lines only, revert bot-check suppression --- .claude/skills/doc-pr/SKILL.md | 18 ++++++++++-------- .github/workflows/claude-doc-pr.yml | 4 ++-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/.claude/skills/doc-pr/SKILL.md b/.claude/skills/doc-pr/SKILL.md index ae24e78429..eb4f31efd1 100644 --- a/.claude/skills/doc-pr/SKILL.md +++ b/.claude/skills/doc-pr/SKILL.md @@ -32,23 +32,25 @@ Split the comma-separated file list into individual file paths for processing. This stage applies the doc-help editing analysis to the PR changes — but non-interactively. You are producing a written review, not having a conversation. -1. Run `gh pr diff $DOC_PR_NUMBER` to get the diff -2. For each changed file, read the full file content -3. Analyze ONLY the added or modified lines (lines starting with `+` in the diff) against these priorities: +**Work from the diff only. Do not read the full file content.** The diff is your entire input for the review. This is intentional: pre-existing issues in unchanged lines are out of scope for a PR review. - **Structure** — Is the document organized so readers can find what they need? Can a reader scanning the page quickly find the section they need? For procedures, can someone follow it step by step? For explanatory content, does it build from simple to complex? +1. The PR diff is already saved at `/tmp/pr-diff.txt` — read it with the Read tool +2. For each changed file, identify the added lines (lines starting with `+`, excluding the `+++` file header) +3. Analyze those added lines against these priorities: - **Clarity** — Is the content easy to understand? Can the reader follow the explanation without having to reread, guess at meaning, or fill in gaps? + **Structure** — Do the added lines fit logically into the document? If they introduce a new section or step, is it in the right place? Can a reader scanning the page find it? - **Completeness** — After reading, can the reader do what they came to do? Are there gaps that would force them to search elsewhere or open a support ticket? + **Clarity** — Are the added lines easy to understand? Can the reader follow without rereading, guessing at meaning, or filling in gaps? + + **Completeness** — Do the added lines leave the reader with unanswered questions? If a new concept, term, or step is introduced, is enough context provided? For each issue found, note: -- The file path and line number +- The file path and line number (use the line number from the diff's `@@` hunk header plus offset) - The priority category (Structure, Clarity, or Completeness) - A specific description of the problem - A concrete suggested fix -Only report issues on lines that were added or modified in this PR. Do not flag preexisting issues. +Only report issues on added lines (`+` lines). Never flag lines prefixed with `-` or ` ` (context lines). **Idiom tagging:** When the editorial review catches an idiom or figurative expression, tag it with `[idiom]` at the start of the bullet so the fixer can identify it and add it to the Vale `Idioms.yml` rule. Example: diff --git a/.github/workflows/claude-doc-pr.yml b/.github/workflows/claude-doc-pr.yml index 0022620e73..7f0716d457 100644 --- a/.github/workflows/claude-doc-pr.yml +++ b/.github/workflows/claude-doc-pr.yml @@ -35,8 +35,8 @@ jobs: run: | MESSAGE=$(gh api repos/${{ github.repository }}/commits/${{ github.event.pull_request.head.sha }} --jq '.commit.message') echo "Latest commit message: $MESSAGE" - if echo "$MESSAGE" | grep -qE '^fix\((vale|dale)\):|^ci: trigger build|^docs: apply .* fixes from PR review'; then - echo "Skipping: commit is from autofix or doc-fixer workflow" + if echo "$MESSAGE" | grep -qE '^fix\((vale|dale)\):|^ci: trigger build'; then + echo "Skipping: commit is from autofix workflow" echo "skip=true" >> "$GITHUB_OUTPUT" else echo "skip=false" >> "$GITHUB_OUTPUT" From d689e5c6c54dde75a472cf243ef75ccdfdfcb2e8 Mon Sep 17 00:00:00 2001 From: jth-nw Date: Fri, 1 May 2026 12:58:05 -0500 Subject: [PATCH 3/3] fix(doc-pr): read full file for context but report issues on added lines only --- .claude/skills/doc-pr/SKILL.md | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/.claude/skills/doc-pr/SKILL.md b/.claude/skills/doc-pr/SKILL.md index eb4f31efd1..26aa2d81c7 100644 --- a/.claude/skills/doc-pr/SKILL.md +++ b/.claude/skills/doc-pr/SKILL.md @@ -32,25 +32,23 @@ Split the comma-separated file list into individual file paths for processing. This stage applies the doc-help editing analysis to the PR changes — but non-interactively. You are producing a written review, not having a conversation. -**Work from the diff only. Do not read the full file content.** The diff is your entire input for the review. This is intentional: pre-existing issues in unchanged lines are out of scope for a PR review. +1. The PR diff is already saved at `/tmp/pr-diff.txt` — read it with the Read tool. Identify the added lines (starting with `+`, excluding `+++` file headers) and note which file each change belongs to. +2. For each changed file, **read the full file content**. This gives you the context needed to judge whether new content fits logically, whether a new term was already defined earlier, and whether a new step assumes knowledge established elsewhere in the document. +3. Using the full file as context, analyze the **added lines only** against these priorities: -1. The PR diff is already saved at `/tmp/pr-diff.txt` — read it with the Read tool -2. For each changed file, identify the added lines (lines starting with `+`, excluding the `+++` file header) -3. Analyze those added lines against these priorities: + **Structure** — Does the new content fit logically in its position? Would a reader scanning the document find it where they expect it? If it introduces a new section or step, is the placement correct relative to surrounding content? - **Structure** — Do the added lines fit logically into the document? If they introduce a new section or step, is it in the right place? Can a reader scanning the page find it? + **Clarity** — Is the new content easy to understand on its own? Does it use a term or concept that hasn't been defined yet in the document? - **Clarity** — Are the added lines easy to understand? Can the reader follow without rereading, guessing at meaning, or filling in gaps? - - **Completeness** — Do the added lines leave the reader with unanswered questions? If a new concept, term, or step is introduced, is enough context provided? + **Completeness** — Does the new content leave the reader with an unanswered question? If it introduces a new concept, term, step, or instruction, does it provide enough context — or does the reader need to look elsewhere? For each issue found, note: -- The file path and line number (use the line number from the diff's `@@` hunk header plus offset) +- The file path and line number - The priority category (Structure, Clarity, or Completeness) - A specific description of the problem - A concrete suggested fix -Only report issues on added lines (`+` lines). Never flag lines prefixed with `-` or ` ` (context lines). +**Report issues only on added lines.** The full file is context for your judgment — not a source of additional issues to flag. Do not report problems on lines that were unchanged in this PR. **Idiom tagging:** When the editorial review catches an idiom or figurative expression, tag it with `[idiom]` at the start of the bullet so the fixer can identify it and add it to the Vale `Idioms.yml` rule. Example: