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"