Skip to content

fix(ci): allow-list Publish trigger events in benchmark.yml instead of excluding 'push' - #2114

Merged
carlos-alm merged 3 commits into
mainfrom
fix/benchmark-decouple-from-every-publish
Jul 16, 2026
Merged

fix(ci): allow-list Publish trigger events in benchmark.yml instead of excluding 'push'#2114
carlos-alm merged 3 commits into
mainfrom
fix/benchmark-decouple-from-every-publish

Conversation

@carlos-alm

Copy link
Copy Markdown
Contributor

Summary

  • Both jobs in benchmark.yml gated on github.event.workflow_run.event != 'push'. That's a negative exclusion, not a positive allow-list of the event types that actually warrant a benchmark run.
  • fix(ci): move dev builds from every push to a daily schedule #2113 changes Publish's dev-build trigger from push to schedule!= 'push' doesn't exclude 'schedule', so once that merges, scheduled dev-build Publish completions would newly pass this filter, and:
    • record-benchmarks would try to download benchmark-files/benchmark-results-json artifacts that don't exist for schedule-triggered Publish runs (only release/workflow_dispatch go through pre-publish-benchmark), failing the job
    • embedding-benchmark would kick off a redundant 2.5+ hour run against the latest stable npm version once a day, for nothing
  • Fix: replace the exclusion with an explicit allow-list (event == 'release' || event == 'workflow_dispatch') so it can't silently drift out of sync with Publish's trigger set again

Why

Direct correctness dependency of #2113 (Publish dev-build scheduling). Also corrects my own earlier issue #2109, which incorrectly assumed this workflow ran the full benchmark suite on every Publish completion — it already didn't, this fix just keeps that true after #2113.

Closes #2109
Depends on #2113 (should merge together or #2113 first)

Test plan

  • YAML parses, actionlint shows no new findings (pre-existing shellcheck notes only)
  • After both PRs merge: verify the first scheduled dev-build Publish completion does not trigger record-benchmarks or embedding-benchmark
  • Verify a real release still triggers both as before

…f excluding 'push'

Both jobs gated on `workflow_run.event != 'push'`, which stops matching once
Publish's dev-build trigger moves from push to schedule (companion PR).
Switch to a positive allow-list of 'release'/'workflow_dispatch' so the
filter can't silently drift out of sync with Publish's trigger set again.
@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR hardens the if: conditions on both jobs in benchmark.yml by replacing a negative event exclusion (!= 'push') with an explicit allow-list (== 'release' || == 'workflow_dispatch'). This is a correctness fix needed before PR #2113 lands, which changes Publish's dev-build trigger from push to schedule.

  • record-benchmarks: now only runs when Publish was triggered by release or workflow_dispatch — the only paths that produce the benchmark-files/benchmark-results-json artifacts this job consumes.
  • embedding-benchmark: same allow-list applied; prevents a spurious 2.5+ hour benchmark run firing daily once schedule becomes a valid Publish trigger.
  • Direct workflow_dispatch of benchmark.yml itself is preserved for the embedding-benchmark job.

Confidence Score: 5/5

Safe to merge; the two-line condition change is logically sound and directly prevents unwanted benchmark runs once #2113 lands.

The change converts a fragile negative exclusion into a tight positive allow-list. Both jobs are updated consistently, the asymmetry between them is intentional and correctly preserved. No artifacts, secrets, or permissions are touched.

No files require special attention; the single changed file has a minimal, well-commented diff.

Important Files Changed

Filename Overview
.github/workflows/benchmark.yml Both record-benchmarks and embedding-benchmark job conditions updated from != 'push' exclusion to `== 'release'

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    PW["Publish workflow completes"]
    BW["benchmark.yml triggered via workflow_run"]

    PW --> |"event = 'release'"| BW
    PW --> |"event = 'workflow_dispatch'"| BW
    PW --> |"event = 'push' (legacy)"| X1["FILTERED — was allowed before fix"]
    PW --> |"event = 'schedule' (PR #2113)"| X2["FILTERED — would have been allowed before fix"]

    BW --> FILTER{"if condition (after fix)"}
    FILTER --> |"event == 'release' OR 'workflow_dispatch'"| JOBS["Run record-benchmarks + embedding-benchmark"]
    FILTER --> |"any other event"| SKIP["Skip — no run"]

    DW["Direct workflow_dispatch on benchmark.yml"] --> |"Only embedding-benchmark"| EMBED["embedding-benchmark runs (record-benchmarks skipped)"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    PW["Publish workflow completes"]
    BW["benchmark.yml triggered via workflow_run"]

    PW --> |"event = 'release'"| BW
    PW --> |"event = 'workflow_dispatch'"| BW
    PW --> |"event = 'push' (legacy)"| X1["FILTERED — was allowed before fix"]
    PW --> |"event = 'schedule' (PR #2113)"| X2["FILTERED — would have been allowed before fix"]

    BW --> FILTER{"if condition (after fix)"}
    FILTER --> |"event == 'release' OR 'workflow_dispatch'"| JOBS["Run record-benchmarks + embedding-benchmark"]
    FILTER --> |"any other event"| SKIP["Skip — no run"]

    DW["Direct workflow_dispatch on benchmark.yml"] --> |"Only embedding-benchmark"| EMBED["embedding-benchmark runs (record-benchmarks skipped)"]
Loading

Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

@carlos-alm

Copy link
Copy Markdown
Contributor Author

Addressed Claude's review feedback: none required — Greptile's review (5/5 confidence, "Safe to merge") did not flag any findings or gaps against the workflow diff itself.

What was done since that review:

@carlos-alm

Copy link
Copy Markdown
Contributor Author

@greptileai

@carlos-alm
carlos-alm merged commit 7aa3811 into main Jul 16, 2026
14 checks passed
@carlos-alm
carlos-alm deleted the fix/benchmark-decouple-from-every-publish branch July 16, 2026 08:50
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 16, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: benchmark.yml's push-exclusion filter will break once Publish's dev-build trigger changes

1 participant