Skip to content

PR checks: keep a findings/remediated table in the status comment #2018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
marcleblanc2 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

PR checks: keep a findings/remediated table in the status comment #2018

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dcdd33a
PR checks: keep a findings/remediated table in the status comment
marcleblanc2 Sep 21, 2026
74a69b1
PR checks: count distinct findings across revisions, not the peak count
marcleblanc2 Sep 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 7 additions & 21 deletions .github/workflows/check-links.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ jobs:
--baseline "$RUNNER_TEMP/base-links.json" \
--diff "$RUNNER_TEMP/changes.diff" \
--review "$RUNNER_TEMP/review.json" \
--keys "$RUNNER_TEMP/keys.txt" \
--link-base "$LINK_BASE" > "$RUNNER_TEMP/report.md"; then
echo "result=clean" >> "$GITHUB_OUTPUT"
elif [ -s "$RUNNER_TEMP/report.md" ]; then
Expand All @@ -85,29 +86,14 @@ jobs:
PR_NUMBER: ${{ github.event.pull_request.number }}
RESULT: ${{ steps.check.outputs.result }}
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
# The status comment keeps a findings/remediated table; see dev/upsert-report-comment.sh
run: |
marker='<!-- check-links-report -->'
existing_comment=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \
--paginate --jq ".[] | select(.body | startswith(\"$marker\")) | .id" | head -n 1)

# Comment when there is something to report, or an earlier report to resolve
case "$RESULT" in
broken)
{ echo "$marker"; cat "$RUNNER_TEMP/report.md"; } > "$RUNNER_TEMP/comment.md" ;;
crashed)
printf '%s\n### ⚠️ The links check could not run on this revision\n\nThis is a problem with the check, not with this PR; see the [job log](%s).\n' \
"$marker" "$RUN_URL" > "$RUNNER_TEMP/comment.md" ;;
*)
[ -n "$existing_comment" ] || exit 0
printf '%s\n### ✅ This revision introduces no broken links\n' \
"$marker" > "$RUNNER_TEMP/comment.md" ;;
esac

if [ -n "$existing_comment" ]; then
gh api --method PATCH "repos/$GITHUB_REPOSITORY/issues/comments/$existing_comment" \
--field body=@"$RUNNER_TEMP/comment.md"
if [ "$RESULT" = crashed ]; then
printf '### ⚠️ The links check could not run on this revision\n\nThis is a problem with the check, not with this PR; see the [job log](%s).\n' \
"$RUN_URL" > "$RUNNER_TEMP/report.md"
dev/upsert-report-comment.sh '<!-- check-links-report -->' "$RUNNER_TEMP/report.md" --crashed
else
gh pr comment "$PR_NUMBER" --body-file "$RUNNER_TEMP/comment.md"
dev/upsert-report-comment.sh '<!-- check-links-report -->' "$RUNNER_TEMP/report.md" "$RUNNER_TEMP/keys.txt"
fi

- name: Suggest fixes as review comments
Expand Down
28 changes: 7 additions & 21 deletions .github/workflows/check-redirects.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ jobs:
--baseline "$RUNNER_TEMP/base-redirects.json" \
--diff "$RUNNER_TEMP/changes.diff" \
--review "$RUNNER_TEMP/review.json" \
--keys "$RUNNER_TEMP/keys.txt" \
--link-base "$LINK_BASE" > "$RUNNER_TEMP/report.md"; then
echo "result=clean" >> "$GITHUB_OUTPUT"
elif [ -s "$RUNNER_TEMP/report.md" ]; then
Expand All @@ -87,29 +88,14 @@ jobs:
PR_NUMBER: ${{ github.event.pull_request.number }}
RESULT: ${{ steps.check.outputs.result }}
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
# The status comment keeps a findings/remediated table; see dev/upsert-report-comment.sh
run: |
marker='<!-- check-redirects-report -->'
existing_comment=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \
--paginate --jq ".[] | select(.body | startswith(\"$marker\")) | .id" | head -n 1)

# Comment when there is something to report, or an earlier report to resolve
case "$RESULT" in
broken)
{ echo "$marker"; cat "$RUNNER_TEMP/report.md"; } > "$RUNNER_TEMP/comment.md" ;;
crashed)
printf '%s\n### ⚠️ The redirects check could not run on this revision\n\nThis is a problem with the check, not with this PR; see the [job log](%s).\n' \
"$marker" "$RUN_URL" > "$RUNNER_TEMP/comment.md" ;;
*)
[ -n "$existing_comment" ] || exit 0
printf '%s\n### ✅ This revision breaks no redirects\n' \
"$marker" > "$RUNNER_TEMP/comment.md" ;;
esac

if [ -n "$existing_comment" ]; then
gh api --method PATCH "repos/$GITHUB_REPOSITORY/issues/comments/$existing_comment" \
--field body=@"$RUNNER_TEMP/comment.md"
if [ "$RESULT" = crashed ]; then
printf '### ⚠️ The redirects check could not run on this revision\n\nThis is a problem with the check, not with this PR; see the [job log](%s).\n' \
"$RUN_URL" > "$RUNNER_TEMP/report.md"
dev/upsert-report-comment.sh '<!-- check-redirects-report -->' "$RUNNER_TEMP/report.md" --crashed
else
gh pr comment "$PR_NUMBER" --body-file "$RUNNER_TEMP/comment.md"
dev/upsert-report-comment.sh '<!-- check-redirects-report -->' "$RUNNER_TEMP/report.md" "$RUNNER_TEMP/keys.txt"
fi

- name: Suggest fixes as review comments
Expand Down
9 changes: 7 additions & 2 deletions .github/workflows/spellcheck.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,14 @@ jobs:
HEAD_REF: ${{ github.event.pull_request.head.ref }}
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
CRASHED: ${{ steps.check.outputs.crashed }}
# The status comment keeps a findings/remediated table; see dev/upsert-report-comment.sh
run: |
if [ "$CRASHED" = true ]; then
node dev/post-spelling-review.mjs --crashed
printf '### ⚠️ The spell check could not run on this revision\n\nThis is a problem with the check, not with this PR; see the [job log](%s).\n' \
"$RUN_URL" > "$RUNNER_TEMP/summary.md"
dev/upsert-report-comment.sh '<!-- cspell-report -->' "$RUNNER_TEMP/summary.md" --crashed
else
node dev/post-spelling-review.mjs --findings "$RUNNER_TEMP/spelling.json"
node dev/post-spelling-review.mjs --findings "$RUNNER_TEMP/spelling.json" \
--summary "$RUNNER_TEMP/summary.md" --keys "$RUNNER_TEMP/keys.txt"
dev/upsert-report-comment.sh '<!-- cspell-report -->' "$RUNNER_TEMP/summary.md" "$RUNNER_TEMP/keys.txt"
fi
8 changes: 7 additions & 1 deletion AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,13 @@ gh api repos/sourcegraph/docs/pulls/<pr>/comments --paginate \

Fork PRs get no comments; the findings are in the job log
(`gh run view <run-id> --log`). The checks resolve their own comments on the
next run.
next run: inline comments are deleted, and the summary comment is updated in
place (`dev/upsert-report-comment.sh`) with a `Total findings` / `Remediated`
table, so a merged PR's comment says how many findings it was merged with.
The table counts distinct findings across every revision (the same broken
link or misspelled word on several lines of one file is one finding), so it
can differ from the count in the heading.
`dev/pr-check-metrics.sh [merged-since]` sums those tables over merged PRs.

### Vercel build failures

Expand Down
14 changes: 14 additions & 0 deletions dev/check-links.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
* --review <file> Write a GitHub pull request review (JSON body for
* POST /repos/{owner}/{repo}/pulls/{n}/reviews) with one
* suggested-change comment per added line that has a fix
* --keys <file> Write the findings' identities, one per line, for
* dev/upsert-report-comment.sh to count across revisions
*
* Exits 1 when any finding is reported.
*/
Expand All @@ -62,6 +64,7 @@ const LINK_BASE = flagValue('--link-base')?.replace(/\/$/, '');
const DIFF = parseDiff(flagValue('--diff'));
const CHECK_EXTERNAL = args.includes('--check-external');
const REVIEW_FILE = flagValue('--review');
const KEYS_FILE = flagValue('--keys');

if (CHECK_EXTERNAL && !DIFF) {
throw new Error('--check-external needs --diff, to know which lines were added');
Expand Down Expand Up @@ -516,6 +519,14 @@ function findingKey({ file, url, error }) {
return `${file}\n${url}\n${error}`;
}

// The --keys file: one JSON string per line, so a key can hold any character,
// with `>` escaped since the lines end up inside an HTML comment on the PR
export function findingKeyLines(keys) {
return [...new Set(keys)]
.map(key => JSON.stringify(key).replaceAll('>', '\\u003e') + '\n')
.join('');
}

function withoutBaseline(findings, baselineFile) {
const baseline = new Set(
JSON.parse(fs.readFileSync(baselineFile, 'utf-8')).map(findingKey)
Expand Down Expand Up @@ -679,6 +690,9 @@ async function main() {
if (REVIEW_FILE) {
fs.writeFileSync(REVIEW_FILE, JSON.stringify(reviewRequest(findings), null, '\t') + '\n');
}
if (KEYS_FILE) {
fs.writeFileSync(KEYS_FILE, findingKeyLines(findings.map(findingKey)));
}
process.stdout.write(format(findings));
process.exit(findings.length === 0 ? 0 : 1);
}
Expand Down
8 changes: 7 additions & 1 deletion dev/check-redirects.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
* diff added get a suggested change
* --review <file> Write a pull request review with one suggested
* change per fixable finding to this JSON file
* --keys <file> Write the findings' identities, one per line, for
* dev/upsert-report-comment.sh to count across revisions
*
* Exits 1 when any finding is reported.
*/
Expand All @@ -38,7 +40,7 @@ import fs from 'fs';
import path from 'path';
import vm from 'vm';
import {fileURLToPath} from 'url';
import {extractHeadings, listFiles, routeFor} from './check-links.mjs';
import {extractHeadings, findingKeyLines, listFiles, routeFor} from './check-links.mjs';

const __dirname = path.dirname(fileURLToPath(import.meta.url));

Expand All @@ -49,6 +51,7 @@ const BASELINE_FILE = flagValue('--baseline');
const LINK_BASE = flagValue('--link-base')?.replace(/\/$/, '');
const DIFF_FILE = flagValue('--diff');
const REVIEW_FILE = flagValue('--review');
const KEYS_FILE = flagValue('--keys');

const REDIRECTS_PATH = 'src/data/redirects.ts';
const REDIRECTS_FILE = path.join(ROOT_DIR, REDIRECTS_PATH);
Expand Down Expand Up @@ -490,6 +493,9 @@ function main() {
JSON.stringify(reviewRequest(findings, added), null, '\t') + '\n'
);
}
if (KEYS_FILE) {
fs.writeFileSync(KEYS_FILE, findingKeyLines(findings.map(findingKey)));
}

// Not process.exit(): that can truncate stdout when it is a pipe
process.stdout.write(format(findings));
Expand Down
128 changes: 35 additions & 93 deletions dev/post-spelling-review.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,30 @@
#!/usr/bin/env node

/**
* Reports CSpell findings on a pull request: one summary comment in the
* discussion, plus an inline review comment on each flagged line. Findings in
* the pull request's title or description (those with a `field`) have no line
* to comment on, so they appear in the summary only. Once the findings are
* fixed, the summary is minimized as resolved and the inline comments are
* deleted; the review that carried them has no body, so nothing of it remains
* visible.
* Reports CSpell findings on a pull request: writes the summary for the PR's
* status comment (posted by dev/upsert-report-comment.sh), and keeps one
* inline review comment on each flagged line. Findings in the pull request's
* title or description (those with a `field`) have no line to comment on, so
* they appear in the summary only. Once the findings are fixed, the inline
* comments are deleted; the review that carried them has no body, so nothing
* of it remains visible.
*
* Usage: node dev/post-spelling-review.mjs --findings <json-file> [--dry-run]
* node dev/post-spelling-review.mjs --crashed [--dry-run]
* Usage: node dev/post-spelling-review.mjs --findings <json-file> \
* --summary <md-file> --keys <keys-file> [--dry-run]
*
* Reads the JSON written by `dev/check-spelling.mjs --format json`. With
* `--crashed`, the summary says the check could not run and links the job log
* (RUN_URL) instead; the inline comments are left as they are.
* Reads the JSON written by `dev/check-spelling.mjs --format json`. Writes
* the summary Markdown and, to the keys file, one identity per finding (file
* and word, since line numbers shift between revisions) for
* dev/upsert-report-comment.sh to count across revisions.
* Requires GH_TOKEN, GITHUB_REPOSITORY, PR_NUMBER, HEAD_SHA and HEAD_REF.
*/

import {readFileSync} from 'fs';
import {readFileSync, writeFileSync} from 'fs';

const args = process.argv.slice(2);
const FINDINGS_FILE = args[args.indexOf('--findings') + 1];
const CRASHED = args.includes('--crashed');
const SUMMARY_FILE = args[args.indexOf('--summary') + 1];
const KEYS_FILE = args[args.indexOf('--keys') + 1];
const DRY_RUN = args.includes('--dry-run');
const MAX_INLINE_COMMENTS = 25;

Expand All @@ -31,12 +33,10 @@ const REPOSITORY = process.env.GITHUB_REPOSITORY;
const PR_NUMBER = process.env.PR_NUMBER;
const HEAD_SHA = process.env.HEAD_SHA;
const HEAD_REF = process.env.HEAD_REF;
const RUN_URL = process.env.RUN_URL;

// Link to the PR branch, not the commit, so GitHub's edit button works from it
const ALLOW_LIST_LINK = `[\`cspell-allow-list.txt\`](https://github.com/${REPOSITORY}/blob/${HEAD_REF}/cspell-allow-list.txt)`;

const SUMMARY_MARKER = '<!-- cspell-report -->';
const INLINE_MARKER = '<!-- cspell-finding:';

async function github(method, route, body) {
Expand Down Expand Up @@ -76,26 +76,6 @@ async function githubWrite(method, route, body) {
}
}

// Minimizing a comment is GraphQL-only. Both mutations are idempotent.
async function setCommentMinimized(nodeId, minimized) {
const mutation = minimized
? 'minimizeComment(input: {subjectId: $id, classifier: RESOLVED}) { clientMutationId }'
: 'unminimizeComment(input: {subjectId: $id}) { clientMutationId }';
console.log(
`${DRY_RUN ? '[dry-run] ' : ''}${minimized ? 'minimize' : 'unminimize'} comment ${nodeId}`
);
if (DRY_RUN) {
return;
}
const result = await github('POST', '/graphql', {
query: `mutation ($id: ID!) { ${mutation} }`,
variables: {id: nodeId}
});
if (result.errors) {
throw new Error(`GraphQL failed: ${JSON.stringify(result.errors)}`);
}
}

function groupByFile(findings) {
const grouped = new Map();
for (const finding of findings) {
Expand Down Expand Up @@ -142,9 +122,13 @@ function summaryItem(finding) {
return `\`${finding.word}\`${suggestion ? ` → \`${suggestion}\`` : ''}`;
}

// The status comment's text. Its first line carries the count for
// dev/upsert-report-comment.sh, which adds the running total below it
function summaryBody(findings) {
if (findings.length === 0) {
return '### ✅ This revision introduces no spelling errors\n';
}
const lines = [
SUMMARY_MARKER,
`### ⚠️ Spell check found ${findings.length} issue(s) in this PR`,
'',
'Only findings on lines added by this PR, and in its title and description, are shown.',
Expand Down Expand Up @@ -173,47 +157,14 @@ function summaryBody(findings) {
return lines.join('\n') + '\n';
}

function crashedBody() {
return [
SUMMARY_MARKER,
'### ⚠️ The spell check could not run on this revision',
'',
`This is a problem with the check, not with this PR; see the [job log](${RUN_URL}).`,
''
].join('\n');
}

const RESOLVED_BODY = `${SUMMARY_MARKER}\n### ✅ This revision introduces no spelling errors\n`;

// Post the summary, or replace the earlier one. A resolved summary is kept,
// collapsed, so the discussion still shows what was flagged and fixed; it is
// reopened when there is something to say again.
async function upsertSummaryComment(body, resolved) {
const comments = await githubList(
`/repos/${REPOSITORY}/issues/${PR_NUMBER}/comments`
);
const existing = comments.find(comment =>
comment.body.startsWith(SUMMARY_MARKER)
);

if (!existing) {
// Nothing to resolve
if (!resolved) {
await githubWrite(
'POST',
`/repos/${REPOSITORY}/issues/${PR_NUMBER}/comments`,
{body}
);
}
return;
}

await githubWrite(
'PATCH',
`/repos/${REPOSITORY}/issues/comments/${existing.id}`,
{body}
);
await setCommentMinimized(existing.node_id, resolved);
// One JSON string per line, as dev/check-links.mjs writes its --keys file:
// any character fits, and `>` is escaped since the lines end up inside an
// HTML comment on the PR. Not imported from there, since this job does not
// install its github-slugger dependency
function findingKeyLines(findings) {
return [...new Set(findings.map(({file, word}) => `${file}\u0000${word}`))]
.map(key => JSON.stringify(key).replaceAll('>', '\\u003e') + '\n')
.join('');
}

function findingKey({file, line, word}) {
Expand Down Expand Up @@ -337,25 +288,16 @@ async function main() {
throw new Error(`Missing required environment variable ${name}`);
}
}
if (CRASHED) {
if (!RUN_URL) {
throw new Error('Missing required environment variable RUN_URL');
}
console.log('Reporting that the check crashed');
await upsertSummaryComment(crashedBody(), false);
return;
}
if (!FINDINGS_FILE) {
throw new Error('Missing required --findings <json-file>');
if (!FINDINGS_FILE || !SUMMARY_FILE || !KEYS_FILE) {
throw new Error(
'Missing required --findings <json-file> --summary <md-file> --keys <keys-file>'
);
}

const findings = JSON.parse(readFileSync(FINDINGS_FILE, 'utf8'));
console.log(`${findings.length} finding(s) to report`);
const resolved = findings.length === 0;
await upsertSummaryComment(
resolved ? RESOLVED_BODY : summaryBody(findings),
resolved
);
writeFileSync(SUMMARY_FILE, summaryBody(findings));
writeFileSync(KEYS_FILE, findingKeyLines(findings));
await syncInlineComments(findings);
}

Expand Down
Loading
Loading