diff --git a/.github/DOC-REVIEW-PIPELINE-PLAN.md b/.github/DOC-REVIEW-PIPELINE-PLAN.md index 60815f485..bc3269854 100644 --- a/.github/DOC-REVIEW-PIPELINE-PLAN.md +++ b/.github/DOC-REVIEW-PIPELINE-PLAN.md @@ -81,11 +81,10 @@ PR opened/updated (content paths) └──────────┬───────────────┘ │ for auto-loaded review rules │ │ └──────────────┬─────────────────┘ ▼ │ -┌─ Job 3: Copilot Visual Review ────────┐ │ +┌─ Job 3: Visual Review check run ──────┐ │ │ Wait for preview deployment │ │ -│ Post preview URLs + review prompt │ │ -│ @copilot analyzes rendered HTML │ │ -│ Checks: layout, shortcodes, 404s │ │ +│ Create GitHub Check Run │ │ +│ Output: preview URLs + checklist │ │ └──────────────┬───────────────────────┘ │ │ │ ▼ ▼ @@ -336,64 +335,52 @@ files. It checks for: - `review:*` labels are applied manually by humans after reviewing the Copilot feedback — the workflow does not manage labels -### 2.4 — Job 3: Copilot Visual Review (rendered HTML) +### 2.4 — Job 3: Visual Review check run (rendered HTML) -**Purpose:** Have Copilot analyze the rendered preview pages to catch visual -and structural issues invisible in the Markdown source. +**Purpose:** Surface visual-review status in the Checks tab and provide +reviewers with the preview URLs and checklist to verify the rendered pages. + +**Background:** The original design posted a PR comment mentioning +`@github-copilot` and expected Copilot Vision to respond. This never +worked — comment mentions alone do not invoke any Copilot agent or API, +and the job produced no entry in the Checks tab. The redesign replaces +the comment approach with a GitHub Check Run created via `checks.create`. **Dependencies:** Depends on Job 1 (needs URL list). Must wait for the `pr-preview.yml` deployment to be live. -**Why Copilot for visual review:** -- Copilot can analyze rendered HTML content at public preview URLs — no - screenshot capture or image upload required. -- Visual review is a good fit for Copilot because the rendered pages are - self-contained artifacts (no need to cross-reference repo files). -- Copilot code review (Job 2) handles the diff; visual review catches what - the diff review cannot. - **Implementation:** -1. **Wait for preview deployment:** +1. **Create an in-progress check run** as soon as the job starts: + - Uses `github.rest.checks.create` with `status: 'in_progress'` + - Appears immediately in the Checks tab as "Visual Review" + - Requires `checks: write` permission + +2. **Wait for preview deployment:** - Poll `https://influxdata.github.io/docs-v2/pr-preview/pr-{N}/` with - `curl --head` until it returns 200 + `curl` until it returns 200 - Timeout: 10 minutes (preview build takes ~75s + deploy time) - Poll interval: 15 seconds - - If timeout, skip visual review; Copilot code review (Job 2) still runs -2. **Post preview URLs and trigger Copilot review:** - - Use `actions/github-script@v7` to post a PR comment listing the preview - URLs from Job 1, formatted as clickable links - - Post a follow-up comment tagging `@copilot` with instructions to review - the rendered pages at the preview URLs. The comment should instruct - Copilot to check each page for: - - Raw shortcode syntax visible on the page (`{{<` or `{{%`) - - Placeholder text that should have been replaced - - Broken layouts: overlapping text, missing images, collapsed sections - - Code blocks rendered incorrectly (raw HTML/Markdown fences visible) - - Navigation/sidebar entries correct - - Visible 404 or error state - - Product name inconsistencies in the rendered page header/breadcrumbs - - The review instruction template is stored in - `.github/prompts/copilot-visual-review.md` for maintainability - - Preview URL count capped at 50 pages (matching `MAX_PAGES` in - `detect-preview-pages.js`) - -3. **Comment upsert pattern:** - - Visual review comments use a marker-based upsert pattern — the workflow - updates an existing comment if one with the marker exists, otherwise - creates a new one. This prevents duplicate comments on `synchronize` - events. +3. **Complete the check run** based on the outcome: + - **Preview available:** `conclusion: 'neutral'` — the check run output + contains the full page list and the visual review checklist from + `.github/prompts/copilot-visual-review.md`. Human reviewers access it + via the Checks tab "Details" link. + - **Timeout:** `conclusion: 'neutral'` — the check run output explains + the timeout and how to re-trigger the review. + - **No URLs:** Job 4 (`report-skipped`) creates a `conclusion: 'skipped'` + check run instead. ### 2.6 — Workflow failure handling -- If preview deployment times out: skip Copilot visual review (Job 3), - Copilot code review (Job 2) still runs independently. Post a comment - explaining visual review was skipped. -- If Copilot does not respond to the `@copilot` mention: the preview URLs - remain in the comment for human review. -- Never block PR merge on workflow failures — the workflow adds comments - but does not set required status checks or manage labels. +- If preview deployment times out: Job 3 completes the check run with + `conclusion: 'neutral'` and a timeout message. Copilot code review + (Job 2) still runs independently. +- If there are no previewable URLs: Job 4 creates a check run with + `conclusion: 'skipped'` explaining why. +- Never block PR merge on workflow failures — the workflow creates check + runs but does not set required status checks or manage labels. --- @@ -420,8 +407,9 @@ CLAUDE.md ← lightweight pointer (already exis - **Copilot code review (CI):** GitHub's native reviewer. Auto-loads instruction files from `.github/instructions/` based on changed file patterns. No custom prompt or API key needed. -- **Copilot visual review (CI):** Triggered by `@copilot` mention in a PR - comment with preview URLs and a review template. +- **Visual review (CI):** A GitHub Check Run created by the + `copilot-visual-review` job. Surfaces the review status in the Checks + tab and includes the preview URLs and checklist in the check run output. - **Claude local review:** Uses `.claude/agents/doc-review-agent.md` for local Claude Code sessions. Not used in CI. - Shared rules (style guide, frontmatter, shortcodes) stay in the existing @@ -507,14 +495,14 @@ Shared definitions for severity levels, comment structure, and result → label mapping. Used by `doc-review-agent.md` (local review sessions) and the Copilot visual review template. -#### Copilot visual review template +#### Visual review check run -The `@copilot` visual review comment is constructed inline in the -`doc-review.yml` workflow using the review template from -`.github/templates/review-comment.md`. Contains: +Job 3 creates a GitHub Check Run via `github.rest.checks.create` and +completes it with the preview URLs and the visual review checklist from +`.github/prompts/copilot-visual-review.md`. Contains: - The visual review checklist (raw shortcodes, broken layouts, 404s, etc.) -- Instructions for analyzing the rendered pages at the preview URLs +- Clickable links to the preview pages - Output format guidance (what to flag, severity levels) --- @@ -546,19 +534,19 @@ These are explicitly **not** part of this plan. Documented here for context. ## Decisions (Resolved) -### Q1: How should Copilot review rendered pages? — RESOLVED +### Q1: How should rendered-page visual review be surfaced? — RESOLVED -**Decision:** Copilot reviews rendered HTML at public preview URLs — no -screenshots needed. Job 3 posts preview URLs in a PR comment, then tags -`@copilot` with a review prompt. See section 2.5 for implementation details. +**Decision:** Job 3 creates a GitHub Check Run via `github.rest.checks.create` +instead of posting a PR comment mentioning `@github-copilot`. A comment +mention alone never invokes Copilot Vision, and it produces no entry in +the Checks tab. The check run: +- Appears in the Checks tab as "Visual Review" with a meaningful conclusion +- Contains the preview URLs and the visual review checklist in its output +- Shows `neutral` when the preview is live (awaiting human review), or + `skipped` when there are no previewable URLs -This approach works because: -- Preview pages are publicly accessible at `influxdata.github.io/docs-v2/pr-preview/pr-{N}/` -- Copilot can analyze HTML content at public URLs -- No screenshot capture, image upload, or artifact management required - -Screenshot capture is deferred to Future Phases (v2) for design/layout PRs -where visual regression testing matters. +Screenshot capture and automated visual-diff tooling are deferred to +Future Phases (v2) for design/layout PRs where visual regression matters. ### Q2: Should the review workflow be a required status check? — RESOLVED @@ -701,4 +689,4 @@ Triggered via `workflow_dispatch` with `pr_number=6890` on branch - Added `workflow_dispatch` with `pr_number` input for on-demand re-runs **Remaining:** Visual review (Job 3) needs a content-changing PR to fully exercise -the preview URL polling and Copilot `@copilot` mention flow. +the preview URL polling and check run creation flow. diff --git a/.github/LABEL_GUIDE.md b/.github/LABEL_GUIDE.md index 14de67670..e3ff90454 100644 --- a/.github/LABEL_GUIDE.md +++ b/.github/LABEL_GUIDE.md @@ -53,7 +53,7 @@ Human approval uses GitHub's native PR review system (CODEOWNERS), not labels. 2. Doc review workflow triggers (unless `skip-review` is present) 3. Copilot code review runs on the diff (uses [`.github/instructions/`](instructions/) files from the base branch) -4. Copilot visual review checks rendered preview pages +4. Visual Review check run reports rendered-page status in the Checks tab 5. Human reviewer uses GitHub's PR review for final approval Review labels (`review:*`) are applied manually after review, not by CI. diff --git a/.github/workflows/doc-review.yml b/.github/workflows/doc-review.yml index 11c807b2a..d1ea24463 100644 --- a/.github/workflows/doc-review.yml +++ b/.github/workflows/doc-review.yml @@ -114,13 +114,19 @@ jobs: } # ----------------------------------------------------------------- - # Job 3: Copilot visual review (depends on Job 1 for URLs) + # Job 3: Visual review check run (depends on Job 1 for URLs) + # + # Creates a GitHub Check Run (visible in the Checks tab) instead of + # posting a PR comment mentioning @github-copilot. A comment mention + # alone never invokes Copilot Vision; only a proper check run surfaces + # the visual-review status where reviewers can act on it. # ----------------------------------------------------------------- copilot-visual-review: runs-on: ubuntu-latest permissions: contents: read - pull-requests: write + pull-requests: read + checks: write needs: resolve-urls if: needs.resolve-urls.result == 'success' && fromJson(needs.resolve-urls.outputs.url-count) > 0 steps: @@ -130,6 +136,47 @@ jobs: sparse-checkout: .github/prompts/copilot-visual-review.md sparse-checkout-cone-mode: false + - name: Get PR head SHA + id: pr-sha + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + env: + PR_NUMBER: ${{ github.event.pull_request.number || inputs.pr_number }} + with: + script: | + let sha = context.payload.pull_request?.head?.sha; + if (!sha) { + const prNumber = context.issue.number || Number(process.env.PR_NUMBER); + const { data: pr } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber, + }); + sha = pr.head.sha; + } + core.setOutput('sha', sha); + + - name: Create in-progress check run + id: create-check + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + env: + HEAD_SHA: ${{ steps.pr-sha.outputs.sha }} + with: + script: | + const { data: check } = await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + name: 'Visual Review', + head_sha: process.env.HEAD_SHA, + status: 'in_progress', + started_at: new Date().toISOString(), + output: { + title: 'Visual Review — Waiting for preview deployment', + summary: 'Waiting for PR preview to be available…', + }, + }); + core.setOutput('check-run-id', String(check.id)); + core.info(`Created check run ${check.id}`); + - name: Wait for preview deployment id: wait env: @@ -157,210 +204,145 @@ jobs: echo "Preview deployment timed out after ${TIMEOUT}s" echo "available=false" >> "$GITHUB_OUTPUT" - - name: Post visual review request + - name: Complete check run — preview available if: steps.wait.outputs.available == 'true' uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 env: PREVIEW_URLS: ${{ needs.resolve-urls.outputs.urls }} PR_NUMBER: ${{ github.event.pull_request.number || inputs.pr_number }} + CHECK_RUN_ID: ${{ steps.create-check.outputs.check-run-id }} with: script: | const fs = require('fs'); - - let urls; - try { - urls = JSON.parse(process.env.PREVIEW_URLS); - } catch (e) { - core.warning(`Failed to parse PREVIEW_URLS: ${e.message}`); - return; - } - + const checkRunId = Number(process.env.CHECK_RUN_ID); const prNumber = context.issue.number || Number(process.env.PR_NUMBER); const previewBase = `https://influxdata.github.io/docs-v2/pr-preview/pr-${prNumber}`; - // Read the Copilot visual review template - const template = fs.readFileSync( + let urls = []; + try { urls = JSON.parse(process.env.PREVIEW_URLS); } catch {} + + const checklist = fs.readFileSync( '.github/prompts/copilot-visual-review.md', 'utf8' ); + const pageList = urls.slice(0, 20) + .map(u => `- [${u}](${previewBase}${u})`) + .join('\n') + + (urls.length > 20 ? `\n- … and ${urls.length - 20} more` : ''); - const marker = ''; - const timestamp = new Date().toISOString().replace('T', ' ').split('.')[0] + ' UTC'; - const body = [ - marker, - '## 🔍 Visual Review — Doc Review Bot', - '', - `| Status | Details |`, - `|--------|---------|`, - `| **Pages** | ${urls.length} page(s) to review |`, - `| **Preview** | See PR Preview comment above for full URL list |`, - `| **Requested** | ${timestamp} |`, - '', - '> **Note:** Preview URLs are listed in the **PR Preview** comment to avoid duplication.', - '> Click the preview link there to view the deployed pages.', - '', - '---', - '', - `@github-copilot please review the ${urls.length} preview pages for this PR using the checklist below:`, - '', - template.trim(), - '', - '### Pages to Review', - '', - ...urls.slice(0, 10).map(u => { - const pageUrl = `${previewBase}${u}`; - return `- \`${u}\` → [preview](${pageUrl})`; - }), - ...(urls.length > 10 ? [`- ... and ${urls.length - 10} more (see PR Preview comment)`] : []), - ].join('\n'); - - // Update existing comment or create new one - const { data: comments } = await github.rest.issues.listComments({ + await github.rest.checks.update({ owner: context.repo.owner, repo: context.repo.repo, - issue_number: prNumber, + check_run_id: checkRunId, + status: 'completed', + conclusion: 'neutral', + completed_at: new Date().toISOString(), + output: { + title: `Visual Review — ${urls.length} page(s) ready for review`, + summary: `Preview is live with ${urls.length} changed page(s). Complete the checklist in the details to finish visual review.`, + text: `## Pages to Review\n\n${pageList}\n\n## Visual Review Checklist\n\n${checklist.trim()}`, + }, }); - const existing = comments.find(c => c.body.includes(marker)); + core.info(`Completed check run ${checkRunId} — ${urls.length} pages ready`); - if (existing) { - await github.rest.issues.updateComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: existing.id, - body, - }); - } else { - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: prNumber, - body, - }); - } - - core.info(`Posted visual review request with ${urls.length} URLs`); - - - name: Post timeout notice + - name: Complete check run — timed out if: steps.wait.outputs.available == 'false' uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 env: PR_NUMBER: ${{ github.event.pull_request.number || inputs.pr_number }} + CHECK_RUN_ID: ${{ steps.create-check.outputs.check-run-id }} with: script: | + const checkRunId = Number(process.env.CHECK_RUN_ID); const prNumber = context.issue.number || Number(process.env.PR_NUMBER); - const marker = ''; - const timestamp = new Date().toISOString().replace('T', ' ').split('.')[0] + ' UTC'; - const body = [ - marker, - '## 🔍 Visual Review — Doc Review Bot', - '', - `| Status | Details |`, - `|--------|---------|`, - `| **Result** | ⏱️ TIMED OUT |`, - `| **Reason** | Preview deployment not available within 10 minutes |`, - `| **Checked** | ${timestamp} |`, - '', - '### What This Means', - '', - '- Visual review was **skipped** because the preview was not deployed in time', - '- Copilot code review (diff-based) still ran independently', - '- Human reviewers should check the preview manually when available', - '', - '### Next Steps', - '', - 'To trigger visual review:', - '1. Wait for the PR Preview to deploy (check the PR Preview comment)', - '2. Re-run this workflow from the Actions tab', - ].join('\n'); + const previewUrl = `https://influxdata.github.io/docs-v2/pr-preview/pr-${prNumber}/`; - const { data: comments } = await github.rest.issues.listComments({ + await github.rest.checks.update({ owner: context.repo.owner, repo: context.repo.repo, - issue_number: prNumber, + check_run_id: checkRunId, + status: 'completed', + conclusion: 'neutral', + completed_at: new Date().toISOString(), + output: { + title: 'Visual Review — Preview deployment timed out', + summary: [ + 'Preview was not available within 10 minutes.', + '', + 'To trigger visual review:', + `1. Wait for the [PR Preview](${previewUrl}) to deploy`, + '2. Re-run this workflow from the Actions tab', + ].join('\n'), + }, }); - const existing = comments.find(c => c.body.includes(marker)); - - if (existing) { - await github.rest.issues.updateComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: existing.id, - body, - }); - } else { - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: prNumber, - body, - }); - } + core.info(`Completed check run ${checkRunId} — timed out`); # ----------------------------------------------------------------- # Job 4: Report when visual review is skipped (no URLs to review) + # + # Creates a GitHub Check Run with conclusion 'skipped' so the Checks + # tab shows an accurate status rather than simply omitting the entry. # ----------------------------------------------------------------- report-skipped: runs-on: ubuntu-latest permissions: - pull-requests: write + pull-requests: read + checks: write needs: resolve-urls # GitHub Actions outputs are always strings; 'true' string comparison is intentional if: needs.resolve-urls.result == 'success' && needs.resolve-urls.outputs.skipped == 'true' steps: - - name: Post skipped notice + - name: Get PR head SHA + id: pr-sha uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 env: PR_NUMBER: ${{ github.event.pull_request.number || inputs.pr_number }} + with: + script: | + let sha = context.payload.pull_request?.head?.sha; + if (!sha) { + const prNumber = context.issue.number || Number(process.env.PR_NUMBER); + const { data: pr } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber, + }); + sha = pr.head.sha; + } + core.setOutput('sha', sha); + + - name: Create skipped visual review check run + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + env: + HEAD_SHA: ${{ steps.pr-sha.outputs.sha }} SKIP_REASON: ${{ needs.resolve-urls.outputs.skip-reason }} with: script: | - const prNumber = context.issue.number || Number(process.env.PR_NUMBER); const skipReason = process.env.SKIP_REASON || 'No previewable content changes detected'; - const marker = ''; - const timestamp = new Date().toISOString().replace('T', ' ').split('.')[0] + ' UTC'; - const body = [ - marker, - '## 🔍 Visual Review — Doc Review Bot', - '', - `| Status | Details |`, - `|--------|---------|`, - `| **Result** | ⏭️ SKIPPED |`, - `| **Reason** | ${skipReason} |`, - `| **Checked** | ${timestamp} |`, - '', - '### What This Means', - '', - '- No preview pages were identified for visual review', - '- This is expected for PRs that only change non-content files', - '- Copilot code review (diff-based) still runs independently', - '', - '### If You Expected Visual Review', - '', - 'Check that your PR includes changes to files in `content/` that map to', - 'published documentation pages.', - ].join('\n'); - - const { data: comments } = await github.rest.issues.listComments({ + const now = new Date().toISOString(); + const { data: check } = await github.rest.checks.create({ owner: context.repo.owner, repo: context.repo.repo, - issue_number: prNumber, + name: 'Visual Review', + head_sha: process.env.HEAD_SHA, + status: 'completed', + conclusion: 'skipped', + started_at: now, + completed_at: now, + output: { + title: 'Visual Review — Skipped', + summary: skipReason, + text: [ + '## What This Means', + '', + '- No preview pages were identified for visual review.', + '- This is expected for PRs that only change non-content files.', + '- Copilot code review (diff-based) still runs independently.', + '', + '## If You Expected Visual Review', + '', + 'Check that your PR includes changes to files in `content/` that map to published documentation pages.', + ].join('\n'), + }, }); - const existing = comments.find(c => c.body.includes(marker)); - - if (existing) { - await github.rest.issues.updateComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: existing.id, - body, - }); - } else { - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: prNumber, - body, - }); - } - - core.info(`Posted visual review skipped notice: ${skipReason}`); + core.info(`Created skipped check run ${check.id}: ${skipReason}`);