GeezerVison

No actionable comments were generated in the recent review.

Merge Risk: Minimal · up to d9a80

The sharded benchmark configuration is mergeable after normal checks.

Pre-merge checks: 4

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title says the CodSpeed benchmarks are sharded across CI jobs instead of running in pytest-xdist worker processes. The raw summary confirms this main change: the codspeed workflow now uses five ma…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title says the CodSpeed benchmarks are sharded across CI jobs instead of running in pytest-xdist worker processes. The raw summary confirms this main change: the codspeed workflow now uses five matrix shards with pytest-split groups, and the test extra swaps pytest-xdist for pytest-split. The title is concise, specific, and follows the conventional-commit style. It names the primary change, so it passes.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: ded2afb2-bb8b-458b-ac8b-4fdf6866e9de

Commits

Reviewing files that changed from the base of the PR and between 2d78e79 and d9a809a.

Files ignored due to path filters: 1

• uv.lock is excluded by !**/*.lock

Files selected for processing: 2

• .github/workflows/codspeed.yml
• pyproject.toml

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (2d78e79) to head (d9a809a).

@@            Coverage Diff            @@
##              main      #669   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           14        14           
  Lines         2495      2495           
  Branches       484       484           
=========================================
  Hits          2495      2495           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

Moving this to draft — the sharding works, but I can't validate CodSpeed's shard aggregation from a fork PR, and I don't want it merged until that's confirmed in-repo. Details:

What works (verified on this PR's CI run):

• The 5 matrix shards split the benchmark set cleanly via pytest-split: 133 / 133 / 133 / 133 / 129 = 661 benchmarks, disjoint and complete, every shard green.
• Each shard uploaded to CodSpeed successfully (Performance data uploaded, tokenless/public-repo auth).

The problem: CodSpeed posted no aggregated report on this PR — no CodSpeed Performance Analysis check, no report comment, ~1h after the shards finished (for comparison, the single-job run on #668 reported within minutes). In the shard logs, each of the 5 uploads got a different CodSpeed run hash, i.e. they were recorded as five separate runs (each with only 1/5 of the benchmarks) rather than one run split five ways — so none is a valid comparison against the all-661 baseline, and nothing aggregates.

What it is not: not an auth/OIDC gap. I checked a repo that shards CodSpeed successfully (django-language-server) and it uses only permissions: contents: read — no id-token: write — so OIDC isn't required for aggregation.

Best read: the combination that fails here is fork PR + sharding. #668 (fork, single job) reported fine; in-repo sharded setups report fine; but CodSpeed doesn't seem to correlate several tokenless uploads from a fork PR's workflow run into one report. The CodSpeed docs say shards aggregate when "split within a single CI workflow" but don't document the fork case, and I couldn't find a public fork-PR sharded example that aggregated.

Why this matters before merging: the CodSpeed workflow also runs on push to main. If sharded aggregation fails there too, the default-branch benchmark gate would go silent (no report at all) — which is worse than the status quo. That needs to be confirmed on a branch inside this repo (where the fork limitation doesn't apply), or on a throwaway push, before this is safe to land.

Suggested next step: if you're open to the sharded approach, the decisive test is pushing this codspeed.yml to an in-repo branch and confirming CodSpeed emits one merged report across the 5 shards. If it does, I'll finish this out; if in-repo sharding has the same issue, the single-job -n auto already on main (#668) is the pragmatic keep, and the only downside there is occasional false-regression flags on the ~136 syscall-dominated benchmarks (measurement artifacts, since those run in a worker process under -n auto).

Happy to carry whichever direction you prefer.

Merging this PR will improve performance by 12.47%

⚡ 1 improved benchmark
✅ 660 untouched benchmarks
⏩ 131 skipped benchmarks¹

Performance Changes

_{Comparing invictustitan2:ci/shard-codspeed-benchmarks (d9a809a) with main (2d78e79)}

Correction — I was wrong above, and I'm un-drafting. CodSpeed's aggregated report did post; it was just slow for the sharded run (~1h+ after the shards finished, vs minutes for a single job). Aggregation across the 5 shards worked fine on this fork PR — the five different run hashes I flagged were a red herring (each upload gets its own hash; CodSpeed still merges them by commit into one report). My earlier "fork + sharding doesn't aggregate" conclusion was premature impatience on my part. Apologies for the noise.

The result is exactly what this PR was for — CodSpeed Performance Analysis is green:

Merging this PR will improve performance by 12.47% — ⚡ 1 improved, ✅ 660 untouched.

The one changed benchmark is the same test_no_terminal_output_produced that #668 flagged as an 11.1% regression: 1,070.1 µs → 951.5 µs (+12.47%). That's the artifact being undone — under -n auto it ran in an xdist worker and measured ~1,070 µs; single-process per shard it measures ~951 µs (its true value). The other 660 are untouched, which is the point: sharding keeps the per-benchmark numbers stable because each shard runs single-process, while still parallelizing across 5 jobs.

So the full picture is now confirmed end-to-end: 5 disjoint shards (133/133/133/133/129 = 661) via pytest-split, aggregated into one report, green, no false regression.

One real caveat still stands: the matrix turns the single Run benchmarks check into Run benchmarks (1..5), so if Run benchmarks is a required status check in branch protection, the required-check names would need updating.

Ready for review whenever you'd like to take a look.