Skip to content

ci: exclude composers-50 export bench from regression gate#148

Merged
wpak-ai merged 2 commits into
masterfrom
fix/benchmark-export-50-gate
Jul 21, 2026
Merged

ci: exclude composers-50 export bench from regression gate#148
wpak-ai merged 2 commits into
masterfrom
fix/benchmark-export-50-gate

Conversation

@clean6378-max-it

@clean6378-max-it clean6378-max-it commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Closes #149
Master started failing performance benchmarks after the error code merge. the only red check was test post export zip composers 50 at about 1.6 times the july baseline. composers 10 was already off the gate. same zip and page cache noise on ubuntu runners, not a slowdown from the api work.

this change adds composers 50 to excluded from gate next to composers 10 and updates the benchmark readme so both export zip benches are documented as observation only. parse search and fingerprint benches stay gated. i checked the failing run artifact locally and the gate passes with this diff.

no export behavior change and no baselines json edit. small ci follow up so master goes green again.

Summary by CodeRabbit

  • Documentation

    • Rewrote CI benchmark exclusion guidance, including when to use slack for intentional baseline recalibration versus adding targeted exclusions.
    • Added an “Export ZIP benchmarks” note explaining environmental timing variability for the affected ZIP export scenarios.
    • Kept the rationale for excluding cache lookup and variance-sensitive benchmarks.
  • Tests

    • Updated benchmark regression expectations to align with the expanded CI exclusion list, covering both ZIP export scenarios.
  • Chores

    • Adjusted the CI regression gate exclusions to include the additional ZIP export benchmark case.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The benchmark regression gate now excludes both small-corpus ZIP export benchmarks. Tests update the expected exclusion set, and benchmark documentation explains the exclusions and targeted maintenance guidance.

Changes

Benchmark gate exclusions

Layer / File(s) Summary
Gate exclusion and validation
scripts/check_benchmark_regression.py, tests/test_check_benchmark_regression.py
Adds test_post_export_zip[composers-50] to the gate exclusion configuration and expected test data, with updated comments.
Benchmark maintenance guidance
benchmarks/README.md
Documents cache and ZIP export exclusions, environmental variance, and targeted EXCLUDED_FROM_GATE or --slack maintenance guidance.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: bradjin8

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: excluding the composers-50 export benchmark from the CI regression gate.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/benchmark-export-50-gate

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@benchmarks/README.md`:
- Line 42: Update the benchmark guidance near the listed EXCLUDED_FROM_GATE
entries to avoid recommending global --slack for an individual flaky benchmark.
Direct users to add a targeted EXCLUDED_FROM_GATE entry, and mention --slack
only when intentionally recalibrating all baselines through
scripts/reduce_baselines.py.

In `@scripts/check_benchmark_regression.py`:
- Line 35: Replace the en dash in the range comment with a standard hyphen,
changing “0.5x–1.2x” to “0.5x-1.2x” so the file passes Ruff RUF003.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 97667680-a38b-4333-8d77-538753e17bd5

📥 Commits

Reviewing files that changed from the base of the PR and between 0c67e59 and 09bd0ab.

📒 Files selected for processing (3)
  • benchmarks/README.md
  • scripts/check_benchmark_regression.py
  • tests/test_check_benchmark_regression.py

Comment thread benchmarks/README.md Outdated
Comment thread scripts/check_benchmark_regression.py Outdated
@clean6378-max-it clean6378-max-it self-assigned this Jul 21, 2026
@clean6378-max-it
clean6378-max-it requested a review from wpak-ai July 21, 2026 17:50
@wpak-ai
wpak-ai merged commit a387328 into master Jul 21, 2026
17 checks passed
@wpak-ai
wpak-ai deleted the fix/benchmark-export-50-gate branch July 21, 2026 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cppa-cursor-browser: exclude export zip benchmarks from flaky regression gate on master

3 participants