Skip to content

Add Quarto review prompt, skip-cp-setup label, and render QA guidance - #81

Merged
d-morrison merged 3 commits into
mainfrom
claude/review-prompt-and-extras
May 26, 2026
Merged

Add Quarto review prompt, skip-cp-setup label, and render QA guidance#81
d-morrison merged 3 commits into
mainfrom
claude/review-prompt-and-extras

Conversation

@d-morrison

Copy link
Copy Markdown
Collaborator

Summary

Bundles four AI-config improvements surveyed from your sibling Quarto/R repos (#3/#4/#6/#7 from the cross-repo survey).

  1. .github/prompts/quarto-review.prompt.md (from rme/bcs) — a reusable Copilot review prompt: Quarto-first checklist (links to source .qmd, cross-refs, code-fold, no hard-coded computed values, citations, no new template-wide deps, validation) with severity-ordered output and an explicit "no findings" statement.
  2. copilot-setup-steps.yml skip-cp-setup label (from rpt) — a PR label that short-circuits the heavy install (checkout only) for metadata- or workflow-only PRs. Adds labeled/unlabeled triggers so toggling re-runs, and pull-requests: read to read labels.
  3. copilot-instructions.md "Quality Assurance / Testing Renders" (from qbt) — instructs the agent to render, verify _site/ output, review the PR preview, and fix rendering issues before requesting review (adapted docs/_site/).
  4. CLAUDE.md render-only-touched note (rme preflight discipline) — render just the touched page while iterating; /render remains the full build.

Notes

  • The review prompt references only files already on main (copilot-instructions.md, CLAUDE.md) so this PR is independent of the path-scoped-instructions PR; it mentions .github/instructions/ in prose without a hard link.
  • No new dependencies; all changes are config/docs.

Test plan

  • Add the skip-cp-setup label to a workflow-only PR → confirm the setup job runs checkout only and skips the install steps; remove the label → confirm full setup re-runs.
  • Confirm quarto-review.prompt.md is offered as a Copilot prompt and produces a severity-ordered review.

🤖 Generated with Claude Code

…idance

Bundles four AI-config improvements surveyed from sibling repos:

- .github/prompts/quarto-review.prompt.md — a reusable Copilot review
  prompt (Quarto-first checklist, severity-ordered output). From rme/bcs.
- copilot-setup-steps.yml — a `skip-cp-setup` PR label that short-circuits
  the heavy install (checkout only) for metadata/workflow-only PRs, with
  labeled/unlabeled triggers and pull-requests: read. From rpt.
- copilot-instructions.md — a "Quality Assurance / Testing Renders"
  section (render, check _site/ output, review preview, fix before
  requesting review). From qbt, adapted docs/ -> _site/.
- CLAUDE.md — render only the touched page while iterating, not the whole
  site (the /render command is for the full build).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 26, 2026 05:15
@claude

claude Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @d-morrison's task in 3m 30s —— View job


PR #81 Review — Add Quarto review prompt, skip-cp-setup label, and render QA guidance

  • Gather context and read changed files
  • Review .github/prompts/quarto-review.prompt.md
  • Review .github/workflows/copilot-setup-steps.yml
  • Review .github/copilot-instructions.md
  • Review CLAUDE.md
  • Synthesize and post findings

.github/workflows/copilot-setup-steps.yml

Medium — paths filter blocks the label feature for its intended use case

pull_request:
  types: [opened, synchronize, reopened, labeled, unlabeled]
  paths:
    - .github/workflows/copilot-setup-steps.yml

The labeled/unlabeled triggers inherit the paths filter, so they only fire when the PR also modifies .github/workflows/copilot-setup-steps.yml. Adding skip-cp-setup to a pure metadata PR (e.g., one that changes only copilot-instructions.md) won't trigger this workflow at all — which contradicts the stated goal ("metadata- or workflow-only PRs don't pay the full setup cost") and the test plan ("Add the skip-cp-setup label to a workflow-only PR → confirm the setup job runs checkout only").

The fix is to split the triggers: keep paths: on push for CI validation of the workflow file, but remove it (or use a broader filter) for pull_request so the label can take effect on any PR:

on:
  workflow_dispatch:
  push:
    paths:
      - .github/workflows/copilot-setup-steps.yml
  pull_request:
    types: [opened, synchronize, reopened, labeled, unlabeled]
    # no paths filter — label applies to any PR

Fix this →

Minor — ${{ }} inline in run: script (line 54)

elif [[ '${{ contains(github.event.pull_request.labels.*.name, 'skip-cp-setup') }}' == 'true' ]]; then

This works because contains() always returns true or false. The more conventional and safer pattern is to pass the value through an env: variable, which is the GitHub-recommended approach to prevent expression injection in general:

- name: Check if setup should be skipped
  id: check_label
  env:
    LABELS_CONTAIN_SKIP: ${{ contains(github.event.pull_request.labels.*.name, 'skip-cp-setup') }}
  shell: bash
  run: |
    if [[ '${{ github.event_name }}' != 'pull_request' ]]; then
      echo "skip=false" >> "$GITHUB_OUTPUT"
    elif [[ "$LABELS_CONTAIN_SKIP" == 'true' ]]; then
      echo "skip=true" >> "$GITHUB_OUTPUT"
    else
      echo "skip=false" >> "$GITHUB_OUTPUT"
    fi

Not blocking — the current approach is safe for a boolean expression — but the env-var pattern avoids the anti-pattern showing up in code reviews of downstream repos that copy this template.


.github/prompts/quarto-review.prompt.md

Minor — comma-separated list of 3 items in item 8 (line 33)

The prompt itself enforces "lists of three or more items use bullets." Item 8 reads:

The author ran the required local validation: render of the touched page, lintr, and spelling::spell_check_package().

That's three comma-separated items. By the repo's own rule (and item 2 of the checklist), this should be a nested bullet list:

8. The author ran the required local validation:
   - render of the touched page
   - `lintr`
   - `spelling::spell_check_package()`

Fix this →

Otherwise the prompt is well-structured. Relative paths (../copilot-instructions.md, ../../CLAUDE.md) resolve correctly from .github/prompts/. The scope and output-format sections are clear and appropriately scoped.


.github/copilot-instructions.md

Clean addition. The four-bullet QA checklist under "Testing Renders" is well-formed (blank line above the list, sub-bullets under "Check the rendered output" are fine as continuation items). The note that quarto render <file>.qmd --to html is used "while iterating" is accurate — reviewers should understand this only validates HTML, not PDF or other configured formats, but that's an appropriate caveat for iteration speed. No issues.


CLAUDE.md

The expanded Local Preview bullet is clear and internally consistent. The /render command reference is appropriate for the Claude Code context. The line is longer than the surrounding bullets but not egregiously so. No issues.


Overall

The review prompt and QA guidance sections are solid and ready to merge. The CLAUDE.md change is clean. The workflow has one real semantic gap (the paths filter blocking the label feature for its intended use case) and one style-level suggestion (env-var pattern). Address the paths issue before merging — the label feature won't work as documented without it.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds AI/configuration documentation and workflow improvements to support Quarto-first reviews and faster Copilot agent setup when full R/Quarto installation isn’t needed.

Changes:

  • Add a reusable Quarto-focused PR review prompt under .github/prompts/.
  • Add a skip-cp-setup PR-label gate to short-circuit heavy install steps in copilot-setup-steps.yml.
  • Add render QA guidance to .github/copilot-instructions.md and single-page render iteration guidance to CLAUDE.md.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
CLAUDE.md Adds guidance to render only the touched page during iteration and reserve full renders for /render / full validation.
.github/workflows/copilot-setup-steps.yml Adds label-based gating to skip heavy setup steps and expands PR event triggers/permissions.
.github/prompts/quarto-review.prompt.md Introduces a Quarto-first PR review prompt/checklist and output format guidance.
.github/copilot-instructions.md Adds a “Quality Assurance / Testing Renders” section describing local render checks and preview verification.

Comment thread .github/workflows/copilot-setup-steps.yml Outdated
Comment thread .github/copilot-instructions.md
Comment thread .github/prompts/quarto-review.prompt.md Outdated
- quarto-review.prompt.md: convert checklist item 8's three comma-separated
  validation steps to a bullet list, per the repo's own bullet-list rule.
- copilot-setup-steps.yml: pass the event name and label-contains expression
  through env vars in the skip check rather than interpolating them into the
  shell, the GitHub-recommended pattern against expression injection.

(The `paths:` filter does not block the skip-cp-setup label: Copilot invokes
copilot-setup-steps directly when provisioning its environment, independent of
the `on:` triggers, so the label is honored there.)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @d-morrison's task in 1m 54s —— View job


PR #81 Review — Add Quarto review prompt, skip-cp-setup label, and render QA guidance

  • Gather context
  • Read .github/workflows/copilot-setup-steps.yml
  • Read .github/prompts/quarto-review.prompt.md
  • Read .github/copilot-instructions.md
  • Read CLAUDE.md
  • Synthesize and post findings

What changed since the last review

Two of the previously flagged issues were fixed in this push:

  • Fixed — The contains() expression is now passed through env: variables (EVENT_NAME, LABELS_CONTAIN_SKIP) rather than interpolated directly in the shell. Clean. The comment on lines 49–51 explaining the why is appropriate here.
  • Fixed — Checklist item 8 in quarto-review.prompt.md now uses nested bullets instead of a comma-separated list.

Remaining issues

Medium — paths filter still blocks the label feature for its intended use case (.github/workflows/copilot-setup-steps.yml, lines 27–28)

This is the same issue from the previous review — it was not addressed. The pull_request trigger still carries the paths filter:

pull_request:
  types: [opened, synchronize, reopened, labeled, unlabeled]
  paths:
    - .github/workflows/copilot-setup-steps.yml

Because paths applies to all event types in a trigger block, the labeled/unlabeled events only fire when the PR also touches .github/workflows/copilot-setup-steps.yml. Adding skip-cp-setup to a metadata-only PR (e.g., one that only modifies copilot-instructions.md) will not trigger this workflow at all.

The fix is the same as before — split the push and pull_request triggers:

on:
  workflow_dispatch:
  push:
    paths:
      - .github/workflows/copilot-setup-steps.yml
  pull_request:
    types: [opened, synchronize, reopened, labeled, unlabeled]
    # no paths filter — label applies to any PR

Fix this →


Minor — .github/instructions/ reference in the prompt file (.github/prompts/quarto-review.prompt.md, line 46)

This was flagged by Copilot and remains unaddressed. The final line reads:

plus any path-scoped rules under .github/instructions/ that apply to the changed files.

That directory does not exist in this repo. An agent following this prompt will look for it and find nothing, which is harmless — but slightly confusing, and the phrasing implies the directory should be there. Adding "if present" (or removing the clause entirely, since there's nothing there) would be cleaner:

Base the review on [copilot-instructions.md](../copilot-instructions.md) and
[CLAUDE.md](../../CLAUDE.md), plus any path-scoped rules under
`.github/instructions/` (if present) that apply to the changed files.

Fix this →


Clarification on the Copilot "blank line" finding

Copilot flagged the nested sub-bullets under "Check the rendered output" in copilot-instructions.md (lines 88–91) as missing a blank line before the nested list. This finding is incorrect for nested lists: adding a blank line between a list item and its continuation sub-items in CommonMark-style Markdown would break the nesting, terminating the parent item. The repo's blank-line rule applies to starting a new top-level bullet list, not to sub-lists within an item. The current formatting is correct.


Summary

.github/copilot-instructions.md, CLAUDE.md, and the checklist body of quarto-review.prompt.md are clean and ready. The workflow has one real semantic gap (paths filter preventing the label from working on non-workflow PRs) and one minor prose fix (conditional qualifier on the .github/instructions/ reference). Address the paths issue before merging.

@d-morrison

Copy link
Copy Markdown
Collaborator Author

On the paths: filter vs the skip-cp-setup label: these are independent, so no code change is needed there. GitHub Copilot's coding agent invokes copilot-setup-steps directly when provisioning its environment for an assigned issue/PR — that invocation is not governed by the on: triggers (the push/pull_request + paths: filter only control when the workflow runs as an ordinary validation job). So the label is honored during Copilot's provisioning regardless of the paths: filter, and keeping paths: correctly limits the validation runs to changes of the setup file itself. (Confirmed against GitHub's coding-agent environment docs; matches the proven d-morrison/rpt setup.) I did apply the env-var contains() pattern and fixed the prompt's comma-list nit in the latest push.

- copilot-setup-steps.yml: document that the workflow has two independent
  invocation paths — Copilot's agent provisioning runs the job directly
  (not via `on:`, so `paths:` does not affect it; the skip-cp-setup label
  applies there) vs. the `on:`-triggered validation CI runs (paths-gated to
  this file). This explains why the paths filter is correct and pre-empts
  the recurring "paths blocks the label" misreading.
- quarto-review.prompt.md: qualify the `.github/instructions/` reference with
  "(if present)" since that directory may not exist in a fresh template copy.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @d-morrison's task in 1m 59s —— View job


PR #81 Re-review — Add Quarto review prompt, skip-cp-setup label, and render QA guidance

  • Gather context and read previous review discussion
  • Read current state of all four changed files
  • Assess which prior findings are resolved
  • Synthesize and post findings

What changed since the last review

All three previously open items are now resolved:

  • Fixed.github/instructions/ reference in quarto-review.prompt.md (line 46) now carries the (if present) qualifier. ✅
  • Accepted (by design) — The paths: filter on the pull_request trigger is now justified by the explanatory comment block (workflow lines 14–25). d-morrison's clarification is accurate: GitHub's Copilot coding agent invokes copilot-setup-steps directly, bypassing the on: triggers entirely, so the paths: gate only affects the validation CI job — which is the intended behaviour. The comment makes this unambiguous for future readers. ✅
  • Previously fixed — env-var pattern for contains() remains correctly applied. ✅

Fresh pass — no new issues

.github/workflows/copilot-setup-steps.yml

Clean. The dual-invocation comment block is clear and accurate. All if: steps.check_label.outputs.skip != 'true' guards are applied consistently to every heavy step. The checkout step is correctly left unconditional (Copilot needs checkout even in the skipped-setup path). pull-requests: read permission is minimal and justified.

.github/prompts/quarto-review.prompt.md

Clean. Item 8 nested bullets are correct. Relative paths (../copilot-instructions.md, ../../CLAUDE.md) resolve correctly from .github/prompts/. The scope, checklist, and output-format sections are well-calibrated for this template repo.

.github/copilot-instructions.md

Clean. The QA / Testing Renders section is well-formed. Blank line before the top-level list at line 86 ✅. Nested sub-bullets under "Check the rendered output" have no blank line before them — correct; a blank line there would break the nesting in CommonMark. Copilot's earlier finding on this was a false positive.

CLAUDE.md

Clean. The expanded Local preview bullet accurately describes per-file rendering for iteration and correctly distinguishes it from the /render full-build command.


Summary

No findings. The PR is clean and ready to merge.

@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://d-morrison.github.io/qwt/pr-preview/pr-81/

Built to branch gh-pages at 2026-05-25 22:33 PDT.
Preview will be ready when the GitHub Pages deployment is complete.

@claude

claude Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @d-morrison's task in 1m 10s —— View job


PR #81 Re-review — Add Quarto review prompt, skip-cp-setup label, and render QA guidance

  • Gather context and read changed files
  • Review .github/workflows/copilot-setup-steps.yml
  • Review .github/prompts/quarto-review.prompt.md
  • Review .github/copilot-instructions.md
  • Review CLAUDE.md
  • Synthesize and post findings

What changed since the last review

Three commits on this branch (9807548, 0baffed, 9e11714). All previously raised issues are resolved:

  • Fixedcontains() expression passed through env: vars (EVENT_NAME, LABELS_CONTAIN_SKIP) rather than interpolated into the shell. ✅
  • Fixed — Checklist item 8 in quarto-review.prompt.md uses nested bullets, not a comma list. ✅
  • Fixed.github/instructions/ reference now qualified with (if present). ✅
  • Accepted (by design)paths: filter on pull_request trigger. The new comment block (lines 14–25) accurately documents that Copilot's agent provisioning invokes copilot-setup-steps directly, bypassing the on: triggers — so the label feature works regardless of the paths gate. The comment makes the dual-invocation model unambiguous for future readers. ✅

Fresh pass — no new issues

.github/workflows/copilot-setup-steps.yml

Clean. The comment block is clear and technically accurate. All six heavy steps (Install system dependencies, Set up Pandoc, Set up R, Install R packages, Set up Quarto, Verify development environment) carry if: steps.check_label.outputs.skip != 'true' guards consistently. checkout is correctly left unconditional. The pull-requests: read permission is minimal and justified.

.github/prompts/quarto-review.prompt.md

Clean. Scope, checklist, and output-format sections are well-calibrated for this template repo. Relative paths (../copilot-instructions.md, ../../CLAUDE.md) resolve correctly from .github/prompts/. Item 8 nested bullets render correctly in CommonMark.

.github/copilot-instructions.md

Clean. Blank line before the top-level bullet list at the section start ✅. Nested sub-bullets under "Check the rendered output" correctly have no blank line before them (a blank line there would break CommonMark nesting). No issues.

CLAUDE.md

Clean. The single-line change correctly distinguishes per-file rendering for iteration from the /render full-build command.


Summary

No findings. The PR is clean and ready to merge.

@d-morrison
d-morrison merged commit 5c0be1c into main May 26, 2026
16 checks passed
@d-morrison
d-morrison deleted the claude/review-prompt-and-extras branch May 26, 2026 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants