Skip to content

Commit c70778d

Browse files
authored
[codex] Validate changelog matrix before sweep reuse (#1853)
1 parent ab43014 commit c70778d

8 files changed

Lines changed: 183 additions & 300 deletions

File tree

.github/workflows/README.md

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -204,20 +204,16 @@ must still contain complete artifacts for the merge run's expected matrix.
204204

205205
The comment is the reuse authorization, so adding it does not trigger or cancel
206206
a PR sweep. Once the comment is present, later commits pushed to a PR with a
207-
full-sweep label do not start another benchmark sweep. GitHub still runs the
208-
CPU-only `check-changelog` job on the new commit before inspecting the reuse
209-
authorization. That job validates the complete YAML/schema, append-only entry
210-
ordering, duplicate YAML keys, byte-for-byte preservation of historical
211-
content, PR links, the generated sweep config, and sweep-label exclusivity.
212-
Link-only correction PRs stop after this CPU check because they have no
213-
benchmark matrix to generate. Only appended entries can continue to the reuse
214-
gate and sweep setup. Removing and re-adding a sweep label explicitly starts a
215-
new sweep.
207+
full-sweep label do not start another benchmark sweep. The synchronize run
208+
checks that the changelog diff can generate the setup matrix before inspecting
209+
the authorization and continuing to setup. This catches malformed conflict
210+
resolutions before reuse can skip setup. Removing and re-adding a sweep label
211+
explicitly starts a new sweep.
216212

217213
`utils/merge_with_reuse.sh <pr-number>` is the supported merge path for reuse.
218214
It merges `main`, preserves the current main changelog bytes, canonicalizes an
219215
appended `XXX` link to the PR URL, pushes a fresh synchronization commit, and
220-
waits for `check-changelog` on that exact SHA before merging.
216+
waits for the PR checks before merging.
221217

222218
On the push-to-main run, `run-sweep.yml` resolves the merged PR from the merge
223219
commit, verifies the source run is an eligible `pull_request` `run-sweep.yml`

.github/workflows/run-sweep.yml

Lines changed: 20 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,18 @@ jobs:
3737
check-changelog:
3838
runs-on: ubuntu-latest
3939
if: >-
40-
github.event_name != 'pull_request' ||
41-
!github.event.pull_request.draft
42-
outputs:
43-
has-additions: ${{ steps.validate.outputs.has-additions }}
44-
metadata-only: ${{ steps.validate.outputs.metadata-only }}
40+
github.event_name == 'pull_request' &&
41+
!github.event.pull_request.draft &&
42+
(
43+
(github.event.action != 'labeled' && github.event.action != 'unlabeled') ||
44+
github.event.label.name == 'sweep-enabled' ||
45+
github.event.label.name == 'full-sweep-enabled' ||
46+
github.event.label.name == 'non-canary-full-sweep-enabled' ||
47+
github.event.label.name == 'full-sweep-fail-fast' ||
48+
github.event.label.name == 'full-sweep-fail-fast-no-canary'
49+
)
4550
steps:
4651
- name: Reject conflicting sweep labels
47-
if: github.event_name == 'pull_request'
4852
env:
4953
SWEEP_LABELS: ${{ toJson(github.event.pull_request.labels.*.name) }}
5054
run: |
@@ -71,27 +75,13 @@ jobs:
7175
with:
7276
fetch-depth: 0
7377

74-
- name: Validate perf-changelog.yaml
75-
id: validate
78+
- name: Validate perf-changelog matrix
7679
run: |
7780
pip install pydantic pyyaml
78-
79-
PR_ARGS=()
80-
if [ "${{ github.event_name }}" = "pull_request" ]; then
81-
BASE_REF="origin/${{ github.base_ref }}"
82-
HEAD_REF="${{ github.event.pull_request.head.sha }}"
83-
PR_ARGS=(--pr-number "${{ github.event.pull_request.number }}")
84-
else
85-
BASE_REF="${{ github.event.before }}"
86-
HEAD_REF="${{ github.event.after }}"
87-
fi
88-
8981
python3 utils/validate_perf_changelog.py \
9082
--changelog-file perf-changelog.yaml \
91-
--base-ref "$BASE_REF" \
92-
--head-ref "$HEAD_REF" \
93-
--github-output "$GITHUB_OUTPUT" \
94-
"${PR_ARGS[@]}"
83+
--base-ref "origin/${{ github.base_ref }}" \
84+
--head-ref "${{ github.event.pull_request.head.sha }}"
9585
9686
reuse-sweep-gate:
9787
needs: check-changelog
@@ -103,7 +93,6 @@ jobs:
10393
if: >-
10494
always() &&
10595
needs.check-changelog.result == 'success' &&
106-
needs.check-changelog.outputs.has-additions == 'true' &&
10796
github.event_name == 'pull_request' &&
10897
github.event.action == 'synchronize' &&
10998
!github.event.pull_request.draft &&
@@ -138,8 +127,10 @@ jobs:
138127
runs-on: ubuntu-latest
139128
if: >-
140129
always() &&
141-
needs.check-changelog.result == 'success' &&
142-
needs.check-changelog.outputs.has-additions == 'true' &&
130+
(
131+
needs.check-changelog.result == 'success' ||
132+
needs.check-changelog.result == 'skipped'
133+
) &&
143134
(
144135
needs.reuse-sweep-gate.result == 'skipped' ||
145136
(
@@ -626,9 +617,9 @@ jobs:
626617

627618
reuse-ingest-artifacts:
628619
needs: setup
629-
# `setup` runs via always() and depends on `check-changelog` plus
630-
# `reuse-sweep-gate`, which is skipped on every push-to-main. Without
631-
# always() here, that skipped gate poisons this job's implicit
620+
# `setup` runs via always() and depends on `check-changelog` and
621+
# `reuse-sweep-gate`, which are skipped on every push-to-main. Without
622+
# always() here, those skipped jobs poison this job's implicit
632623
# success() check and it is skipped even when setup succeeded with
633624
# reuse-enabled=true — silently breaking the merge-time ingest. Guard
634625
# explicitly on setup success instead (same pattern as

.github/workflows/test-changelog-gate.yml

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -63,36 +63,3 @@ jobs:
6363
utils/test_validate_reusable_sweep_artifacts.py \
6464
utils/changelog_gate_tests/test_run_sweep_gating.py \
6565
-v
66-
67-
- name: Test historical push validation
68-
run: |
69-
expect_rejected() {
70-
local label="$1"
71-
shift
72-
local output
73-
if output="$("$@" 2>&1)"; then
74-
echo "::error::${label} was unexpectedly accepted"
75-
return 1
76-
fi
77-
echo "${label} was rejected as expected"
78-
printf '%s\n' "$output"
79-
}
80-
81-
python utils/validate_perf_changelog.py \
82-
--changelog-file perf-changelog.yaml \
83-
--base-ref d99c824b1c4f0b1b007631191657e458ef2a332c^ \
84-
--head-ref d99c824b1c4f0b1b007631191657e458ef2a332c
85-
86-
expect_rejected "PR #1767 malformed merge state" \
87-
python utils/validate_perf_changelog.py \
88-
--changelog-file perf-changelog.yaml \
89-
--base-ref 7b9843d3a6e1fe7a2d92d327e25aae57ed3506c5^ \
90-
--head-ref 7b9843d3a6e1fe7a2d92d327e25aae57ed3506c5 \
91-
--pr-number 1767
92-
93-
expect_rejected "PR #1798 invalid merge state" \
94-
python utils/validate_perf_changelog.py \
95-
--changelog-file perf-changelog.yaml \
96-
--base-ref 8aaff9fbb645ae9df9ba7593fa63273f839b62fb^1 \
97-
--head-ref 8aaff9fbb645ae9df9ba7593fa63273f839b62fb \
98-
--pr-number 1798

KLAUD_DEBUG.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,11 @@ python3 -c "import yaml; yaml.safe_load(open('perf-changelog.yaml'))"
3333

3434
Do **not** try a 3-way merge of `perf-changelog.yaml` — whitespace edits will silently re-trigger the deletion check.
3535

36-
After committing and pushing the resolution, wait for `check-changelog` on the
37-
new head SHA. `/reuse-sweep-run` skips setup and GPU jobs only after this
38-
CPU-only validation succeeds. `utils/merge_with_reuse.sh <PR>` performs this
39-
wait automatically.
36+
After committing and pushing the resolution, the synchronize run checks the
37+
changelog with the same matrix processor used by setup, then checks the reuse
38+
authorization. This catches deleted history or malformed appended entries
39+
before reuse can skip setup. `utils/merge_with_reuse.sh <PR>` performs the push
40+
and waits for the PR checks automatically.
4041

4142
---
4243

0 commit comments

Comments
 (0)