Port qwt CI improvements (link/DOI/char checks, summary, skip-cp-setup) - #827
Conversation
…setup) Bring over qwt's CI advantages that rme lacked: - check-links.yml + lychee.toml — lychee broken-link checker (weekly cron + "links checked by hand" override label; opens an issue on main failures). Template-only URL excludes dropped. - check-bibliography-dois.yml + scripts/check-bibliography-dois.R — validate DOIs in references.bib. - check-non-standard-chars.yaml + scripts/check-non-standard-chars.py — flag smart-quotes / non-ASCII that break LaTeX/PDF output. - summary.yml — AI one-paragraph summary comment on newly opened issues. - copilot-setup-steps.yml — add the `skip-cp-setup` label short-circuit so metadata/workflow-only PRs skip the heavy JAGS/renv/Quarto install. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Claude finished @d-morrison's task in 7m 11s —— View job Code Review — PR #827: Port qwt CI improvements
SummaryThis PR cleanly ports five useful CI features from the qwt template. The overall structure is sound, but there are one confirmed security/reliability bug, one artifact that should not be committed, and several lower-severity issues worth addressing before merge. 🔴 Bug — Shell injection / broken command in
|
- Remove check-links.yml + lychee.toml + check-non-standard-chars.yaml +
its script: on rme's existing content these fail on a large pre-existing
backlog (99 non-standard chars across 44 files; link-check also needs
submodule checkout). Out of scope for this port; revisit as a dedicated
cleanup if wanted.
- Add .claude/commands/{render,lint,check,spell,new-chapter}.md, adapted to
rme (render = single chapter to HTML, not the full book; new-chapter wired
to _quarto-book.yml + _subfiles). Un-ignore .claude/commands/ in .gitignore
(overrides the .claude/* and *.md ignore rules) so they're tracked.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Claude finished @d-morrison's task in 3m 27s —— View job Code Review — PR #827: Port qwt CI improvements
SummaryThe five CI features port cleanly and the 🔴 Bug — Shell injection in
|
check-bibliography-dois fails on rme's references.bib for the same reason as epi204: many book entries legitimately lack a DOI and some doi.org lookups 403. Same pre-existing-backlog reason the link/char checks were dropped. Remaining clean additions: summary, skip-cp-setup, and the .claude slash commands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Claude finished @d-morrison's task in 2m 32s —— View job Code Review — PR #827: Port qwt CI improvements
SummaryAfter commit 🔴 Bug — Shell injection in
|
- summary.yml: pass the AI response via the existing "$RESPONSE" env var
instead of interpolating ${{ steps.inference.outputs.response }} into a
single-quoted shell arg (a quote in the output broke the step / was a
shell-injection vector). Add a system-prompt preamble telling the model
to treat the issue title/body as untrusted data.
- Remove the orphaned __pycache__/*.pyc accidentally committed alongside the
(since-removed) char-check script; add **/__pycache__/ and *.pyc to
.gitignore so it can't recur.
(The review's note on `Bash(quarto render:*)` is a false positive: `:*` is
the documented trailing-wildcard form and matches `quarto render <args>`,
consistent with qwt's settings.json — left as-is.)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Claude finished @d-morrison's task in 2m 41s —— View job Code Review — PR #827: Port qwt CI improvements
SummaryCommit ✅ Fixed since last review
🟡 Bug —
|
|
Addressed the review in c683614:
|
…comment - summary.yml: wrap issue title/body in ===BEGIN/END ISSUE=== markers and skip the comment step on empty model output. - copilot-setup-steps.yml: correct the header comment — the skip-cp-setup label only short-circuits on pull_request events; non-PR invocations (incl. Copilot's direct provisioning, which isn't a documented pull_request event) always run full setup. Treat the label as a CI-validation control rather than a Copilot-provisioning short-circuit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Claude finished @d-morrison's task in 3m 50s —— View job Code Review — PR #827: Port qwt CI improvements
What was fixed
The overall structure is sound after these fixes. One medium-severity finding remains that I'm re-raising with new evidence, and one minor note. 🟡
|
…mmands Use the space-separated trailing-wildcard form (equivalent to `:*`) to match the convention already in .claude/settings.json, per review on #827. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Claude finished @d-morrison's task in 2m 31s —— View job Code Review — PR #827: Port qwt CI improvements
SummaryAll findings from prior review rounds are resolved. The changeset is clean. One informational note from the previous round is still open but not blocking. ✅ All prior findings resolved
ℹ️ Note —
|
Ports the qwt template's CI / AI-config advantages that rme was missing.
Added
summary.yml— AI one-paragraph summary comment on newly opened issues.copilot-setup-steps.yml— adds theskip-cp-setuplabel short-circuit (withpull-requests: read+labeled/unlabeledtriggers) so metadata/workflow-only PRs skip the heavy JAGS/SymPy/renv/Quarto install. rme's setup steps are otherwise unchanged..claude/commands/{render,lint,check,spell,new-chapter}.md— slash commands adapted to rme:render= single chapter to HTML (not the full book),new-chapterwired to_quarto-book.yml+_subfiles, plus aspellcommand..gitignoreupdated to track.claude/commands/(overrides rme's.claude/*and*.mdignore rules).Dropped after first CI run (pre-existing backlog)
All three content-scanning checks failed on rme's existing content, so they were removed from this port (worth a dedicated cleanup PR later):
check-non-standard-chars— 99 non-standard chars across 44 files.check-links+lychee.toml— broken links + needs submodule checkout.check-bibliography-dois— many book entries legitimately lack DOIs; some doi.org 403s.Deliberately not included
lint-project.yamland branch rulesets (skipped by request).Test plan
summary+ the copilot-setup-steps validation run pass.skip-cp-setuplabel makes copilot-setup-steps run checkout-only./render,/lint,/check,/spell,/new-chapteravailable in a Claude session.🤖 Generated with Claude Code