From 3901c11d690cf7c4e087b509da22737bd7473e93 Mon Sep 17 00:00:00 2001 From: Marcus Kainth Date: Fri, 25 Sep 2026 14:31:41 +0100 Subject: [PATCH] ci: file an issue when a nightly line regressed native regress's findings are rendered by scripts/native-regression-issues.sh in the read-only job, one directory per finding with a title, labels and a body. gh cannot fill a .yml issue form, so the body follows the form by hand: each field's label as a ### heading and a render: field fenced in its language. A correctness finding uses 1b-native-divergence.yml (labels divergence and area: native), a cost finding 3-performance.yml (performance and area: native). native-record gains issues: write and files each rendered issue. It still runs no repository code: the rendering happened in the read-only job and the filing step only passes the files to gh. A title ends with "(metric at commit)", and an issue whose title ends the same way, open or closed, is not filed again. --label is dropped silently when the token cannot apply it, so each issue's labels are read back and a mismatch fails the step. workflow_dispatch's inject_regression input shows the filing path working without a real regression. It judges one more line: a copy of the last measured line, tonight's or the history's, under the commit name self-test- with its first refusal one tic earlier. The judge step fails unless regress reports it, and the issue says it was injected. The copy is not in the night's lines, so nothing records it. --- .github/workflows/nightly.yml | 88 +++++++++++++--- CONTRIBUTING.md | 6 +- DEVELOPING.md | 12 ++- scripts/native-regression-issues.sh | 153 ++++++++++++++++++++++++++++ 4 files changed, 240 insertions(+), 19 deletions(-) create mode 100755 scripts/native-regression-issues.sh diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index a9de790..2dac0c6 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -7,9 +7,10 @@ # # native-regression runs `native diff --record` on each main commit not yet # recorded and judges each line against the one before it. native-record -# appends the lines to the `regression-data` branch. Cost is judged only -# between a parent and its child measured one after the other in the same -# job, never against a number from another runner. +# appends the lines to the `regression-data` branch and files an issue for +# each regression. Cost is judged only between a parent and its child +# measured one after the other in the same job, never against a number from +# another runner. # # Throughput is not benchmarked here. A shared runner cannot give the timing a # quiet machine gives. Run `bench-canonical-throughput` on a quiet box instead. @@ -32,6 +33,10 @@ on: max_commits: description: "Main commits native-regression walks after the last recorded one" default: "8" + inject_regression: + description: "Also judge a copy of the night's last line with its refusal one tic earlier, and file what that finds" + type: boolean + default: false concurrency: group: nightly-${{ github.ref }} @@ -132,8 +137,27 @@ jobs: CLICKDOOM_DATABASE: regress run: scripts/native-regression-walk.sh "$RUNNER_TEMP/history.jsonl" regression - name: Judge the night + env: + INJECT: ${{ github.event.inputs.inject_regression || 'false' }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} run: | - if [ ! -s regression/night.jsonl ]; then + judged=regression/night.jsonl + if [ "$INJECT" = true ]; then + # The last measured line, tonight's or the history's, under a + # commit that is not one, with its first refusal one tic earlier. + cat "$RUNNER_TEMP/history.jsonl" regression/night.jsonl \ + | jq -c 'select(.error == null)' | tail -n 1 \ + | jq -c --arg commit "self-test-$GITHUB_RUN_ID" \ + '.commit = $commit | .first_refused_tic = ((.first_refused_tic // 2001) - 1)' \ + >regression/injected.jsonl + if [ ! -s regression/injected.jsonl ]; then + echo "::error::no measured line to inject a regression into" + exit 1 + fi + cat regression/night.jsonl regression/injected.jsonl >regression/judged.jsonl + judged=regression/judged.jsonl + fi + if [ ! -s "$judged" ]; then echo "nothing walked, so nothing to judge" exit 0 fi @@ -142,13 +166,19 @@ jobs: history=(--history "$RUNNER_TEMP/history.jsonl") fi status=0 - "$RUNNER_TEMP/clickdoom-judge" native regress regression/night.jsonl "${history[@]}" \ - || status=$? - case "$status" in - 0) ;; - 3) echo "::warning::a recorded line regressed; the lines above name it" ;; - *) exit "$status" ;; - esac + "$RUNNER_TEMP/clickdoom-judge" native regress "$judged" "${history[@]}" \ + --findings regression/findings.jsonl || status=$? + if [ "$status" -ne 0 ] && [ "$status" -ne 3 ]; then + exit "$status" + fi + if [ "$INJECT" = true ] \ + && ! jq -e --arg commit "self-test-$GITHUB_RUN_ID" 'select(.commit == $commit)' \ + regression/findings.jsonl >/dev/null; then + echo "::error::the injected regression was not found" + exit 1 + fi + cat "$RUNNER_TEMP/history.jsonl" "$judged" >regression/lines.jsonl + scripts/native-regression-issues.sh regression/findings.jsonl regression/lines.jsonl regression/issues - name: Upload what the night found if: ${{ !cancelled() }} uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 @@ -158,12 +188,14 @@ jobs: if-no-files-found: error native-record: - # Writes the branch, and runs no repository code: it checks nothing out, - # and reads only the artifact native-regression uploaded. + # Writes the branch and the issues, and runs no repository code: it + # checks nothing out, and reads only the artifact native-regression + # uploaded. needs: native-regression runs-on: ubuntu-latest permissions: contents: write # appends to the regression-data branch + issues: write # files an issue for each regression timeout-minutes: 10 env: GH_TOKEN: ${{ github.token }} @@ -220,3 +252,33 @@ jobs: -f ref="refs/heads/$REGRESSION_BRANCH" -f sha="$commit" fi echo "recorded $count line(s) on $REGRESSION_BRANCH" + - name: File an issue for each regression + run: | + shopt -s nullglob + failed=0 + for dir in regression/issues/*/; do + title=$(cat "$dir/title") + # The title ends with the metric and the commit in parentheses. + marker="(${title##*(}" + filed=$(gh issue list --repo "$REPO" --state all --limit 20 \ + --search "\"$marker\" in:title" --json number,title \ + | jq -r --arg marker "$marker" '[.[] | select(.title | endswith($marker)) | .number] | first // empty') + if [ -n "$filed" ]; then + echo "already filed as #$filed: $title" + continue + fi + labels=() + while IFS= read -r label; do + labels+=(--label "$label") + done <"$dir/labels" + url=$(gh issue create --repo "$REPO" --title "$title" --body-file "$dir/body.md" "${labels[@]}") + echo "filed $url: $title" + # A label the token may not apply is dropped without an error. + want=$(sort "$dir/labels" | paste -sd, -) + got=$(gh issue view "$url" --json labels --jq '[.labels[].name] | sort | join(",")') + if [ "$got" != "$want" ]; then + echo "::error::$url carries labels '$got', not '$want'" + failed=1 + fi + done + exit "$failed" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8661b34..6fb63bc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -49,9 +49,9 @@ differential smoke. It is necessary and not sufficient. The nightly deep-diff is the only run that compares memory, and no pull request makes it. The nightly also diffs native -mode against the reference emulator on each new main commit, and names the -commit whose first refusal or first divergence moved earlier, or whose tic -statement got slower than its parent's. +mode against the reference emulator on each new main commit, and files an issue +naming the commit whose first refusal or first divergence moved earlier, or +whose tic statement got slower than its parent's. Check by exit code. A pipeline reports only its last command's status, so `make gates | tail` can hide a failure. diff --git a/DEVELOPING.md b/DEVELOPING.md index 7087e62..aab40da 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -195,9 +195,15 @@ last recorded commit again first. The `native-record` job appends the new lines to `native/bench/regression/results.jsonl` on the `regression-data` branch, which -is never merged into main. It is the only job with write access, and it runs no -repository code. With `max_commits` at 0 the walk measures only the last -recorded commit. +is never merged into main. It files an issue from `1b-native-divergence.yml` for +a correctness regression and from `3-performance.yml` for a cost one, once per +metric and commit. It is the only job with write access, and it runs no +repository code. + +Dispatching the nightly with `inject_regression` judges one more line: a copy of +the last measured line, with its refusal one tic earlier, under the commit name +`self-test-`. The issue it files says so. The copy is not recorded. +With `max_commits` at 0 the walk measures only the last recorded commit. The walk runs locally against `make up`: `scripts/native-regression-walk.sh HISTORY OUT`, with `HISTORY` a copy of the diff --git a/scripts/native-regression-issues.sh b/scripts/native-regression-issues.sh new file mode 100755 index 0000000..7532eb6 --- /dev/null +++ b/scripts/native-regression-issues.sh @@ -0,0 +1,153 @@ +#!/usr/bin/env bash +# +# Renders each regression `clickdoom native regress --findings` wrote as an +# issue body, in the shape GitHub gives an issue filed through the form. +# +# scripts/native-regression-issues.sh FINDINGS LINES OUT +# +# FINDINGS is the --findings file. LINES holds every recorded line either +# side of a finding names, history and the night together. For finding N, +# OUT/N/ gets `title`, `labels` (one per line) and `body.md`. A correctness +# finding follows .github/ISSUE_TEMPLATE/1b-native-divergence.yml and a cost +# finding .github/ISSUE_TEMPLATE/3-performance.yml, with each field's label +# as a `###` heading. +# +# A title ends with the metric and the commit in parentheses, which is what +# the filing job searches for before filing, so a regression is filed once. +# A finding whose commit starts with `self-test-` was injected on purpose, +# and its title and body say so. +# +# Environment: +# RUN_URL the workflow run, linked from the body +# SEARCH_TICS the first diff's span, for the reproduction +set -euo pipefail +cd "$(dirname "$0")/.." + +if [ "$#" -ne 3 ]; then + echo "usage: $0 FINDINGS LINES OUT" >&2 + exit 2 +fi +findings=$1 +lines=$2 +out=$3 +run_url=${RUN_URL:-} +search_tics=${SEARCH_TICS:-2000} +wad_sha=$(cut -d' ' -f1 rom/wad/doom1.wad.sha256sum) + +# The recorded line for COMMIT, the last one if it was measured twice. +line_of() { + jq -c --arg commit "$1" 'select(.commit == $commit)' "$lines" | tail -n 1 +} + +# The form's "What disagrees" option for a field named as `kind slot n field`. +what_disagrees() { + case "$1" in + mobj\ *) echo "A mobj field" ;; + sector*|line_side\ *|button\ *|anim\ *) echo "A sector or sector thinker field" ;; + player\ *|psprite\ *|input\ *) echo "A player field" ;; + hud\ *) echo "Status bar or message state" ;; + *rndindex) echo "prndindex or rndindex" ;; + *) echo "$1" ;; + esac +} + +field() { + printf '### %s\n\n%s\n\n' "$1" "${2:-_No response_}" +} + +fenced() { + local fence='```' + printf '### %s\n\n%s%s\n%s\n%s\n\n' "$1" "$fence" "$2" "$3" "$fence" +} + +mkdir -p "$out" +n=0 +while IFS= read -r finding; do + n=$((n + 1)) + dir="$out/$n" + mkdir -p "$dir" + kind=$(jq -r .kind <<<"$finding") + commit=$(jq -r .commit <<<"$finding") + against=$(jq -r .against <<<"$finding") + metric=$(jq -r .metric <<<"$finding") + before=$(jq -r .before <<<"$finding") + after=$(jq -r .after <<<"$finding") + line=$(line_of "$commit") + parent_line=$(line_of "$against") + marker="($metric at ${commit:0:12})" + note="" + if [[ $commit == self-test-* ]]; then + marker="($metric at $commit)" + note="This finding was injected by the nightly's \`inject_regression\` input to show that filing works. Nothing regressed. Close it." + fi + rom_sha=$(git show "$commit:rom/PINNED_HASH" 2>/dev/null || cat rom/PINNED_HASH) + clickhouse=$(jq -r '.clickhouse // "unknown"' <<<"$line") + + { + if [ -n "$note" ]; then + printf '%s\n\n' "$note" + fi + printf 'Filed by the nightly native regression walk%s.\n\n' "${run_url:+ in $run_url}" + } >"$dir/body.md" + + if [ "$kind" = correctness ]; then + case "$metric" in + first_divergent_tic) + tic=${after%% *} + what=$(what_disagrees "$(jq -r '.first_divergent_field // ""' <<<"$line")") + ;; + *) + tic=$(jq -r .first_refused_tic <<<"$line") + what="The first refused tic, which is not a field" + ;; + esac + echo "native: divergence at tic $tic $marker" >"$dir/title" + printf 'divergence\narea: native\n' >"$dir/labels" + refused=$(jq -r .first_refused_tic <<<"$line") + repro="git checkout $commit +make up build-clickdoom gen-probe-trace +trace=refemu/reference_traces/demo3/probe.\$(cut -c1-12 rom/PINNED_HASH).tsv +./target/release/clickdoom native load --fresh +./target/release/clickdoom native diff $search_tics --probe \"\$trace\"" + if [ "$refused" != null ] && [ "$refused" -gt 1 ]; then + repro="$repro +./target/release/clickdoom native diff $((refused - 1)) --probe \"\$trace\"" + fi + { + field "ROM sha256" "$rom_sha" + field "WAD sha256" "$wad_sha" + field "ClickHouse version" "$clickhouse" + field "First divergent tic (gametic)" "$tic" + field "First divergent frame, if a frame differs" "" + field "What disagrees" "$what" + fenced "The divergence report" text "$metric moved from $before at $against to $after at $commit. + +This commit's line: +$line + +The line it was judged against: +$parent_line" + field "Random draws at that tic" "" + fenced "Reproduction" shell "$repro" + } >>"$dir/body.md" + else + ratio=${after##*(} + echo "perf: $metric up ${ratio%)} on its parent $marker" >"$dir/title" + printf 'performance\narea: native\n' >"$dir/labels" + { + field "What kind" "A change made it slower" + field "Which benchmark" "The nightly native regression walk: \`scripts/native-regression-walk.sh\`, then \`clickdoom native regress\`" + fenced "The numbers" text "$metric: $before at $against, $after at $commit. +Both measured in the same job on the same runner, ClickHouse $clickhouse. + +This commit's line: +$line + +Its parent's line: +$parent_line" + field "The machine, and how quiet it was" "$(jq -r '.runner_cpu // "unknown"' <<<"$line"), a GitHub-hosted runner. Parent and child ran one after the other in the same job, with nothing else in it." + field "What you think is happening" "" + } >>"$dir/body.md" + fi +done <"$findings" +echo "$n issue(s) rendered into $out"