Rebalance the overloaded general CI shard after optimization issue #4356

[Trecek]

Summary

The primary Tests workflow currently has two test shards, execution and general. The split was introduced in #2320/#2324 when the single test job took about 5m40s, with the goal of keeping the CI critical path below roughly five minutes.

That balance has drifted:

Ubuntu run	execution test step	general test step	general job
30133679370, merge-group validation immediately before PR #4345	1m57s	5m17s	6m41s
30137753678, merge-group validation of PR #4345	2m06s	11m34s	13m22s
30143142340, recent merge group	2m05s	10m56s	12m25s
30144208946, recent PR	2m06s	11m47s	13m16s

Treat three total test runners as the default topology:

• execution: unchanged
• one positive-select child of the current general shard
• general: the remaining catch-all

PR #4345 is the change associated with the runtime regression. Optimization issue #4356 — Memoize skill capability classification by semantic input is the gating fix. Do not lock the final directory assignment until #4356 is completed and the optimized suite is reprofiled. The current tests/recipe + tests/docs candidate is a starting point, not a fixed requirement.

If post-optimization evidence shows that three total runners cannot keep every test step below five minutes with reasonable headroom, this issue may use four total runners instead: unchanged execution, two positive-select children, and the catch-all general.

The three-runner target produces three Ubuntu test jobs and six Ubuntu/macOS jobs for stable-target pull requests and direct pushes to stable. The permitted four-runner fallback produces four and eight respectively. The current workflow does not apply the macOS expansion to merge-group events.

Current behavior

.github/workflows/tests.yml currently defines:

• execution: tests/execution, contracts, core, planner, pipeline, migration, integration, plus root tests/test_*.py
• general: all of tests/, minus the execution paths and root test files

Each shard runs task test-check with four xdist workers. On PRs targeting develop, conservative path filtering is applied inside the already-scoped shard. Other events, including merge groups, run the full shard.

The five-minute threshold is policy/history rather than an encoded timeout. It is documented in #2320. The repository does not persist per-test timing data or use an automated shard balancer; the current directory assignment has not been rebalanced since May.

Findings

The imbalance is in test execution, not setup

Recent execution test steps remain close to two minutes, while general is now close to eleven or twelve minutes. Both jobs spend roughly the same time installing dependencies, so setup does not explain the gap.

PR #4345 introduced a discrete runtime regression

Comparing the adjacent merge-group validations immediately before PR #4345 and for the PR itself:

• passing test count increased only from 22,968 to 22,996 (+28, about 0.12%)
• pytest duration increased from 312.83s to 689.64s (+376.81s, about 120.45%)

This establishes a sharp step change coincident with PR #4345. Optimization issue #4356 contains the causal investigation and proposed fix. Added test count alone cannot explain the change, and sharding must not substitute for completing that optimization.

tests/recipe is the natural load and failure boundary

A current local JUnit profile of the intended general paths produced the following aggregate testcase time after excluding root tests/test_*.py records:

Partition	Collected records	Summed testcase time	Share
Pre-optimization recipe candidate (tests/recipe + tests/docs)	6,246	563.2s	56.7%
Pre-optimization catch-all remainder	17,420	429.7s	43.3%

tests/recipe alone contributed 5,459 records and 561.9 summed seconds; tests/docs contributed 787 records and only 1.3 seconds. Test count is misleading here: the proposed recipe shard has fewer cases but more measured work.

Recipe is also a coherent diagnostic boundary. Its schema, semantic-rule, bundled-YAML, backend-composition, and dispatch-readiness tests commonly fail together for one recipe defect. The diagnostic profile's eight failures were all different assertions of the same remediation-fable validity problem. tests/recipe/conftest.py also provides module-scoped loaded-recipe fixtures and shared construction helpers.

Keep the directory intact. A file-level bin-packing split can get close to 50/50, but it would separate rule tests from bundled-recipe assertions that exercise the same rules, increase duplicated job-level collection/import setup, and require fragile filename exceptions whenever recipe tests are added.

A recipe child must include a conservative-filter sentinel

A recipe-only positive shard can collect zero selected tests on an unrelated develop PR after conservative filtering, causing pytest's no-tests exit.

If post-optimization timing retains recipe as a positive child, tests/docs/test_doc_counts.py is the least expensive existing sentinel. It is always injected by conservative filtering when the full docs directory is not selected. Adding the inexpensive tests/docs directory to that child:

• guarantees that the shard retains a conservative-filter sentinel
• remains functionally reasonable for recipe/documentation validation
• changes the measured balance only slightly (docs totaled about 1.3 summed seconds)

In the three-runner candidate, the catch-all retains arch, infra, and hooks, while execution already retains contracts, so all three shards have conservative always-run coverage. A four-runner topology must distribute additional existing always-run/sentinel coverage across both positive children and prove the resulting shard/filter intersections in tests; it must not add a runner that can exit with no tests.

Recommendation

Sequence the work:

1. Complete optimization issue #4356.
2. Reprofile the optimized full general shard using clean, repeated measurements.
3. Choose the smallest topology that gives every test step a credible sub-five-minute result with headroom.

Default to three total runners:

shard: [execution, <positive-child>, general]

Permit four total runners without reopening this issue if the post-optimization evidence requires it:

shard: [execution, <positive-child-a>, <positive-child-b>, general]

The current recipe + docs candidate remains the preferred three-runner starting point because it preserves the recipe failure domain and supplies a conservative-filter sentinel. Its membership must be recalculated against post-optimization timing rather than copied mechanically.

Use explicit positive lists for every non-catch-all child and preserve general as the complement. Use an explicit case branch and fail on an unknown shard name; do not allow an unknown value to fall through to the catch-all.

Keep these existing behaviors:

• root test files belong only to execution
• lint-imports runs only on execution
• task test-check, xdist settings, marker exclusion, experimental-feature environment, and conservative filter activation are identical across shards
• filter artifact names retain both OS and shard, which is already collision-safe
• new top-level test directories automatically land in general

No change should be needed in Taskfile.yml, the filter algorithm, or the test-filter manifest.

Expected effect and limit

The pre-optimization data cannot select the final topology. Using the 651-second CI run only as an explanatory anchor, the additive testcase-time model allocates:

• roughly 369 seconds to the proposed recipe + docs shard
• roughly 282 seconds to the proposed catch-all

These are allocation estimates, not scheduling measurements. Summed testcase time does not model xdist packing, collection, fixture behavior, or CI variance; actual shard wall time must be measured in Actions.

Before optimization, three total runners do not support a promise that both general-child test steps will be below five minutes. Even a mathematically perfect two-way division of the current general work bottoms out at 325.5 seconds before duplicated collection/setup overhead.

That is why the optimization ticket is the gate for this issue. After the regression is corrected, a three-runner topology is expected to be sufficient, but the issue deliberately authorizes a fourth total runner if repeated post-optimization CI measurements show otherwise.

Use a planning target below 270 seconds per test step so normal CI variance does not immediately cross the 300-second ceiling. Accept three runners when the post-optimization partition meets that target or repeated Actions runs demonstrate adequate equivalent headroom. Otherwise use four.

Avoid splitting tests/recipe by filename until profiling proves it is necessary. The most balanced filename-based candidates cross functional and fixture boundaries and are harder to maintain.

Structural guard changes

Update tests/infra/test_ci_shard_config.py. The current guard checks existence and broad assignment but does not prove disjointness or correct variable-cardinality routing.

Add assertions that:

• the chosen matrix contains execution, general, and either one or two explicitly named positive children
• the final topology decision is three or four total runners, not an accidental duplicate catch-all
• execution directory and root-file ownership is unchanged
• explicit shard directory sets exist, contain tests, and are pairwise disjoint
• every repository-convention test_*.py file resolves to exactly one shard
• a ratchet rejects pytest's alternate *_test.py filename form unless shard ownership is explicitly extended to support it
• general ignores execution paths, every positive-child path, and root files
• a synthetic new top-level test directory resolves to general
• for an unrelated conservative change, each shard's actual collected file set has a nonempty intersection with build_test_scope()
• empty-change and FullRunReason/fail-open cases remain nonempty and shard-bounded
• unknown shard values fail
• lint-imports remains execution-only
• artifact names include both matrix axes

Acceptance criteria

• Optimization issue #4356 is completed before final shard membership is selected.
• A post-optimization timing table records the evidence used to choose three or four total runners.
• execution continues to collect exactly its current directories and root test files.
• The selected positive child or children are justified by post-optimization runtime and functional-cohesion evidence.
• general remains a catch-all and does not duplicate files owned by any positive child or execution.
• An executable shard/filter-intersection test proves conservative filtering cannot leave any matrix shard with a no-tests exit on an unrelated develop PR.
• Every repository test_*.py file is pairwise disjoint and collectively exhaustive across the selected shards, and a ratchet prevents unsupported *_test.py files.
• Ordinary events create three Ubuntu jobs by default or four when evidence selects the fallback.
• Stable-target PRs and direct stable pushes create six jobs for the three-runner topology or eight for the four-runner topology, without silently changing merge-group OS policy.
• On conservative-filtered develop PRs, filter-stat artifacts remain available with unique OS/shard names.
• At least three representative post-change Actions runs record each shard's test-step and total-job duration.
• Every test step remains below 300 seconds in those runs; target 270 seconds or comparable demonstrated headroom when selecting the topology.
• The PR reports the change in critical-path time and runner minutes relative to runs 30133679370, 30137753678, and 30143142340.
• Any claim that the five-minute target is restored is supported by repeated CI evidence rather than the local additive model.

Cost and rollout notes

• The three-runner target increases ordinary test jobs from two to three and stable-target PR/direct-stable-push jobs from four to six.
• The permitted four-runner fallback increases them to four and eight respectively.
• Each additional job duplicates dependency installation and job-level collection/setup, so aggregate runner time is expected to increase and must be measured after rollout even though critical-path latency should fall.
• The current artifact naming already includes ${os}-${shard} and needs no conceptual redesign.
• A timing sidecar would require additional failure-preserving shell and artifact logic. Track that observability work separately; this issue should use existing Actions step/job durations for rollout evidence.

Data-quality note

The local JUnit run was used only for relative allocation. It unintentionally collected root test files as well; those 572 records and 3.8 summed seconds were excluded from the table above. One collection-skip record had an empty classname and was classified from its name. The run also observed eight related remediation-fable recipe failures, so it is not a green-gate result. Final validation must use clean CI runs.

Requirements

REQ-SHRD-001: The execution shard must retain its current directory set and exclusive ownership of root tests/test_*.py files.

REQ-SHRD-002: The test matrix must support three total shards by default and four total shards when post-optimization evidence demonstrates that the three-shard topology lacks sufficient sub-five-minute headroom.

REQ-SHRD-003: Every explicit child shard must be pairwise disjoint, and the general shard must remain the complement that owns every otherwise-unassigned repository-convention test file.

REQ-SHRD-004: Unknown shard identifiers must fail path computation instead of falling through to the general catch-all.

REQ-FILT-001: Every selected shard topology must retain at least one collected test after intersection with conservative filter scope for unrelated changes.

REQ-FILT-002: Empty-change and fail-open filter outcomes must remain nonempty and bounded by each shard's configured collection paths.

REQ-FILT-003: New top-level test directories must default to the general catch-all unless deliberately assigned to an explicit child.

REQ-CI-001: All shards must preserve the existing task test-check, xdist, marker, feature-environment, and filter-activation behavior.

REQ-CI-002: Import linting must continue to run exactly once on the execution shard.

REQ-CI-003: Filter-stat artifact names must remain unique across operating-system and shard matrix axes on conservatively filtered develop pull requests.

REQ-CI-004: Stable-target pull requests and direct stable pushes must expand the selected shard topology across Ubuntu and macOS without changing merge-group operating-system policy.

REQ-PERF-001: Optimization issue #4356 must be completed before final shard membership is selected.

REQ-PERF-002: The three-versus-four-runner decision must be supported by post-optimization timing evidence from at least three representative Actions runs.

REQ-PERF-003: Every selected shard must remain below 300 seconds of test execution in the representative runs, with a 270-second planning target or equivalent demonstrated variance headroom.

REQ-PERF-004: The implementation pull request must report critical-path and aggregate runner-time changes against the historical baseline runs identified in this issue.

REQ-GUARD-001: Structural tests must prove that all supported test filename forms are assigned exactly once across the selected shards.

REQ-GUARD-002: Structural tests must reject unsupported *_test.py files unless shard ownership is explicitly extended for that pytest filename form.