fix(ci): make refresh dispatch explicit about evals and experiments - #362
mattrossman wants to merge 6 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
mattrossman
changed the title
| branch: chore/refresh-eval-results-${{ github.ref_name }}-${{ github.event_name }}-${{ join(fromJSON(needs.prepare.outputs.suite), '-') }} | ||
| # The action resets an existing branch, so a shared name would drop earlier results. | ||
| # https://github.com/peter-evans/create-pull-request/blob/5f6978faf089d4d20b00c7766989d076bb2fc7f1/src/create-or-update-branch.ts#L297-L310 | ||
| branch: chore/refresh-eval-results-${{ github.ref_name }}-${{ github.run_id }} |
There was a problem hiding this comment.
I might be missing something, but I think this also changes the nightly schedule right?
Before, each night reused the same branch, so an incomplete night's PR was replaced the next day. With the run ID in the name, I think an incomplete night would leave its PR open and the next night would open a new one. It hasn't happened yet, since the recent scheduled runs all completed and auto-merged.
Would it be worth adding the run ID only for manual dispatches? Something like:
branch: chore/refresh-eval-results-${{ github.ref_name }}-${{ github.event_name == 'workflow_dispatch' && github.run_id
|| github.event_name }}No strong opinion if open PRs piling up is fine in practice.
| type: boolean | ||
| required: false | ||
| default: false | ||
| commit_to_branch: |
There was a problem hiding this comment.
Small thing about the PR description: for "keep adding to one refresh PR, dispatch on its branch with commit_to_branch=true", I think merge=true might be needed too. Without it, export-results.ts seems to write only the current run's results to the file, so the earlier ones on that branch would be replaced.
Maybe worth mentioning in the description or in this input's description? I could be misreading the export step.
Rodriguespn
left a comment
Rodriguespn
left a comment
There was a problem hiding this comment.
Approving with two comments. Please read them before merging. Thank you for working on this!
This branch has not been deployed
2 participants
Dispatching a refresh for specific
experimentshas failed several times since July (Jul 16, Jul 24, Jul 28, Aug 7, Aug 14, Aug 26, Sep 9, Sep 30), and keeps coming up on Slack. This causes false alarms on runs and wasted sandboxes.Manual refresh dispatches now run only what you ask for:
experimentsrun only in suites they belong to. Each listed experiment used to be planned in every suite the eval allows. A mixed list now plans just the 2 valid pairs.suiteandexperiment_suitestart blank, and the run fails fast unless you name evals (suiteoreval) and experiments (experiment_suiteorexperiments). The default made sense when refreshes were mainly handled by AI team and mostly for the public benchmark, but now the benchmark is rarely refreshed and multiple teams work in the repo. A dispatch with onlyevalset andsuiteblank still plans its pairs.create-pull-requestresets an existing branch, so a shared name let a later run wipe an earlier run's results. To keep adding to one refresh PR, dispatch on its branch withcommit_to_branch=true.Closes AI-1005