CI: split generator specs into setup-safe subshards by justin808 · Pull Request #4746 · shakacode/react_on_rails

justin808 · 2026-07-18T13:06:56Z

Why

Generator specs dominate the gem workflow's full-matrix critical path. Six post-merge full-matrix runs established a 2,427-second median critical path and a 4,660-second paired runner-sum, while unit jobs remained near two minutes.

What

Preserve the optimized and full-matrix selector contracts from CI: route gem generator specs by changed paths #4691.
Split generator coverage into three deterministic subshards per dependency level.
Build each manifest from the current head's stable RSpec scoped IDs.
Keep context-hook setup groups atomic and balance examples plus shared setup weight.
Fail closed on empty, duplicate, lost, or duplicated selections.
Avoid line numbers, caches, secrets, timing databases, and cross-head state.

Verification

Exact reviewed head: 27d81b4beadad20b74301f17ee53c7f2000ad773.
Hosted-CI safety contract passed.
actionlint passed.
Relaxed yamllint passed with only the repository's existing line-length warnings.
CI change-detector suite passed: 83/83.
CI-equivalent OSS RuboCop passed: 241 files, no offenses.
Exact-head manifest contained 1,277 unique examples.
Shard counts: 426 / 425 / 426.
Setup-safe units: 297 / 298 / 297.
Modeled work: 469 / 470 / 469.
All three grouped RSpec dry-runs completed with zero failures.
Fresh Sol/xhigh exact-head adversarial review regenerated and invoked every selector and found no actionable correctness regressions.
Fresh hosted review cutoff has zero unresolved threads and zero must-fix findings.

The review sandbox's attempted full shard execution could not write its home/cache and hit npm registry DNS ENOTFOUND; no full local shard pass is claimed. .agents/bin/validate --changed was intentionally stopped during the long generator surface and is not claimed green.

Hosted benchmark

Two exact-head force-full attempts passed all 16 matrix rows with zero flakes:

Median critical generator path: 14m34s, down 25m53s (64.0%) from the 2,427s baseline median.
Critical-path attempt-to-attempt spread: 32s (3.7%).
Mean generator runner-sum: 80m58s, up 4.2% from the paired baseline.
Runner-sum attempt-to-attempt spread: 1.8%.

The repeated evidence supports retaining the three-way split. Full job links and calculations are recorded on #4683.

Workflow change audit

Triggers, permissions, action versions, and secret usage are unchanged.
Both dependency levels and both unit rows remain covered.
fail-fast: false remains.
No cache or artifact reuse was added.
The partition is computed from the exact checked-out head, so stale-base or cross-head results cannot be reused.
Changelog: not user-visible; no entry required.

Review churn

The first review rejected leaf-example hashing because it would repeat expensive before(:all)/after(:all) setup across shards. The final design uses setup-safe atomic units and was revalidated after rebasing over the latest main.

The final simplify pass consolidated scoped-ID construction and precomputed unit weights. The portable manifest-read loop was retained deliberately. Those behavior-preserving changes were committed separately and re-reviewed at the exact head above.

Part of #4683. Keep the issue open pending exact-head force-full timing evidence and the required post-merge secondary-event replay.

coderabbitai · 2026-07-18T13:07:10Z

Walkthrough

Generator specs are split across three deterministic CI shards. The workflow partitions examples without separating shared setup, while hosted CI safety tests validate matrix membership, stable sharding, coverage, and generator/unit exclusion behavior.

Changes

Generator CI sharding

Layer / File(s)	Summary
Generator matrix shard definitions `.github/workflows/gem-tests.yml`, `.github/workflows/hosted-ci-safety.test.cjs`	Full and pull-request matrices now use `generators-1`, `generators-2`, and `generators-3` alongside the unit shard.
Setup-safe generator partitioning `.github/workflows/gem-tests.yml`, `.github/workflows/hosted-ci-safety.test.cjs`	Generator examples are grouped by shared context-hook setup, assigned deterministically with weighted balancing, validated for complete unique coverage, and executed through shard-specific manifests.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GemWorkflow
  participant RSpecManifest
  participant GeneratorShard
  GemWorkflow->>RSpecManifest: rebuild generator example manifest
  RSpecManifest->>GeneratorShard: assign setup-safe examples to generators-1..3
  GeneratorShard->>GemWorkflow: write shard-specific manifest
  GemWorkflow->>GeneratorShard: run selected generator examples

Possibly related issues

shakacode/react_on_rails#4683 — Both changes address generator CI sharding and deterministic partitioning.

Possibly related PRs

shakacode/react_on_rails#4691 — Both update gem-test generator routing and hosted CI safety assertions.

Suggested labels: ready-for-hosted-ci

Suggested reviewers: alexeyr-ci2

🚥 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 accurately summarizes the main change: splitting generator specs into setup-safe subshards in CI.

✨ 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 codex/4683-generator-replay

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.

❤️ Share

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

claude · 2026-07-18T13:12:00Z

Review: CI — split generator specs into setup-safe subshards

Overview

Splits the generators gem-test shard (previously one monolithic bundle exec rspec spec/react_on_rails/generators job) into three deterministic subshards (generators-1/2/3). Each shard job independently runs an RSpec --dry-run over the generator specs, walks the example-group tree to keep any group with a before(:context)/after(:context) hook (and its descendants) as one atomic "setup-safe" unit, then greedily bin-packs those units across 3 shards balanced by (example count + setup-hook count), tie-broken by a content hash of the unit id so all three jobs converge on the same partition independently. The corresponding hosted-ci-safety.test.cjs matrix/regex assertions are updated to match.

Strengths

Deterministic, stateless partitioning: the final sort key [-unit_weight, SHA256(unit_id), unit_id] doesn't depend on file-load/iteration order, so the three independently-run shard jobs should converge on an identical partition without sharing any cache/state — a genuinely solid design for the "no cross-head state" requirement called out in the PR description.
Fails closed: empty/duplicate/lost/duplicated-selection checks (abort on non-zero dry-run status, empty manifest, duplicate scoped IDs, lost-or-duplicated examples, empty shard) mean a partitioning bug is much more likely to hard-fail CI than to silently under-test.
Keeps before(:all)/after(:all) setup blocks atomic within a shard, correctly avoiding the previously-rejected leaf-hash approach that would have repeated expensive setup across shards (per the PR's "Review churn" note).
Test updates (hosted-ci-safety.test.cjs) assert both matrix shape and several structural properties of the embedded script (stable IDs, atomic setup handling, deterministic tie-break, no line-number selection, no leaf-hash splitting).

Issues / suggestions (also left as inline comments)

Private RSpec API dependency (hooks.send(:all_hooks_for, position, :context), gem-tests.yml:264): all_hooks_for is a private method on RSpec::Core::Hooks::HookCollections, not part of RSpec's public/semver-covered API. Gemfile.lock currently resolves rspec-core (~> 3.13), so a routine patch/minor bump could rename or change this method's signature and break every hosted CI run using the generator matrix. It fails loudly rather than silently mis-partitioning, but it's a fragile, easy-to-forget maintenance trap — worth a comment noting the dependency, and/or a tighter rspec-core pin.
3x redundant dry-run cost (gem-tests.yml:233-327): each of the three generators-N jobs independently re-runs the entire generator-spec dry-run (loading/grouping all ~1,277 examples) just to keep 1/3 of the result. This partially offsets the critical-path savings the PR is chasing. Computing the manifest once (e.g. in detect-changes) and passing the three .opts files down via artifacts would avoid the repeated group-tree traversal — worth at least measuring whether this overhead is negligible relative to actual spec runtime.
Duplicated shard-count constant: shard_count=3 (gem-tests.yml:229) is a third place encoding "3 subshards," alongside the two jq matrix blocks above it. Low risk at this scale, but nothing ties them together, so a future change to the shard count could easily update only one/two of the three spots.
Test coverage is structural, not behavioral: the new hosted-ci-safety.test.cjs assertions (assertMatches/assertDoesNotMatch) check that certain substrings/regexes appear in the embedded Ruby script text, not that the partitioning algorithm actually produces a correct, balanced, non-overlapping partition for a given spec tree. That behavior is only exercised in hosted CI itself (per the PR's verification notes: 1,277 examples, 426/425/426 split, zero dry-run failures). A small local Ruby unit test (extracting the partitioning logic against a fixture example-group tree) would give faster, more meaningful regression coverage than regex-matching the workflow YAML, and would survive refactors that rename variables without changing behavior.

Security / correctness

No secrets, external network calls, or new permissions are introduced. The script only reads local spec files and writes to RUNNER_TEMP. No line-number-based or cache-based selection is used, consistent with the PR's stated invariants. I did not independently execute the workflow in this sandbox (network/filesystem restrictions here prevent running bundle exec rspec and actionlint), so I'm relying on the PR's stated verification (actionlint, yamllint, 83/83 change-detector suite, RSpec dry-runs) for the parts I couldn't re-run myself.

Overall: solid, well-reasoned design for a genuinely tricky problem (splitting specs with shared expensive setup while keeping shard assignment deterministic and stateless). The main actionable items are documenting/mitigating the private-API dependency and considering whether the 3x dry-run overhead is worth eliminating before merge.

claude · 2026-07-18T13:20:30Z

Review: split generator specs into setup-safe subshards

Overview

This PR splits the generators shard of gem-tests.yml into three deterministic subshards (generators-1/2/3) computed at job-start time from an RSpec --dry-run of the current head. A greedy bin-packing algorithm groups examples into "setup-safe atomic units" (walking the example-group tree to find groups owning before(:context)/after(:context) hooks so shared, expensive setup is never split across shards), balances load by example-count + hook-cost weight, and fails closed on any empty/duplicate/lost/duplicated selection. The hosted-ci-safety.test.cjs suite is extended with matrix-shape assertions and pattern checks on the new embedded script.

Code quality

The fail-closed philosophy is applied consistently: empty manifest, duplicate scoped IDs, lost/duplicated examples, empty shard, and multi-unit hook conflicts all abort loudly rather than degrading silently. This is the right default for a correctness-sensitive CI change.
Partition determinism is sound: the final sort key ([-unit_weight, SHA256(unit_id), unit_id]) and the greedy assignment loop depend only on content-derived data, not on RSpec's discovery/iteration order — so three independently-run jobs (one per shard) converge on an identical global partition and each just returns its own slice. That's a subtle but important property for this design to be correct, and it holds up under inspection.
The existing_units.length > 1 abort guard in the hook-atomicity walk appears unreachable given the strictly top-down traversal (an ancestor is always visited, and thus assigned, before any descendant) — harmless as defensive code, but worth a one-line comment noting it's a safety net rather than a reachable path, or removing it if truly dead.
Left as comments in the code, but worth restating: this correctness depends on HookCollections#all_hooks_for (a private RSpec API) continuing to behave as expected. See inline comment for a suggested fast smoke-test to catch RSpec-version drift early instead of only during a full hosted generator run.

Potential issues / risks

Private API dependency (inline comment on gem-tests.yml:263-267): reasonable given RSpec has no public hook-enumeration API, but the failure mode is expensive to discover (only surfaces after a full dry-run in each shard job). A tiny isolated smoke test would catch drift faster.
Duplicated shard_count = 3 magic number (inline comment on gem-tests.yml:230) across two matrix-building blocks and the run step, with no single source of truth — misconfiguration fails loudly (good) but is still a three-site edit hazard.
Matrix shard value rename (generators → generators-1/2/3): this changes the derived GitHub check-run name for this job. If branch protection's required status checks reference the old literal check name anywhere outside this repo (GitHub UI settings, not visible in-repo), this could unexpectedly block merges until updated. I checked ci-required.yml / script/ci-required-hosted-gate and neither hardcodes per-shard check names — this repo appears to gate on a single aggregator check, which mitigates the risk — but it's worth a maintainer double-check of the actual branch protection config before merging, since I can't inspect that from here.
Minor nit: the dry-run output sink (File.open(File::NULL, "w")) is never closed. Harmless in a short-lived process, but easy to wrap in a begin/ensure or just not worth flagging further.

Test coverage

New hosted-ci-safety.test.cjs assertions verify matrix shape (3 generator rows × 2 dependency levels in full-matrix mode, 3 rows in optimized PR mode) and confirm specific implementation patterns exist/don't exist in the embedded script (private hook API used, no line-number selection, no leaf-hash splitting). These are useful regression guards against previously-rejected designs (per PR description's "review churn" section) but are pattern/text checks, not behavioral tests — they can't catch a logic bug in the bin-packing or atomicity-detection code itself. See inline comment for a suggested fixture-based follow-up.
The PR description is transparent that a full local shard execution and .agents/bin/validate --changed were not completed (sandbox networking limitations), so real behavioral confidence rests on the claimed dry-run validation (1,277 examples, 426/425/426 shard split, zero dry-run failures) plus upcoming hosted CI runs.

Security

No new permissions, secrets, actions, or triggers. The embedded Ruby is a static single-quoted string with no PR-controlled interpolation into the shell command — no injection surface introduced.

Summary

Solid, well-reasoned design for a genuinely hard problem (deterministic, no-cache, per-head sharding while respecting before(:context) boundaries). The main things worth addressing before/after merge are tightening the private-API blast radius (fast smoke test), removing the duplicated shard-count constant, and eventually backing the new sharding logic with a behavioral (not just pattern-matching) test.

justin808 · 2026-07-18T13:23:39Z

Address-review summary

Scan scope: full PR history; no previous address-review cutoff.

Mattered

Private RSpec API dependency: documented explicitly in 1f5c104d9; exact-head force-full latest/minimum execution remains a merge gate. Both related threads were replied to and resolved.
Per-job manifest recomputation: retained deliberately to keep each result bound to its exact checkout and avoid cross-job artifacts/state; local generation is sub-second versus the measured 25–40 minute suite baseline. Thread replied to and resolved.
Shard-count duplication: synchronization comment added; a broader matrix-schema rewrite was declined because mismatch already fails closed and the fixed topology is contract-tested. Both related threads were replied to and resolved.
Fixture extraction / required-gate RSpec smoke: declined as scope and gate expansion. Two independent exact-head reviews executed the embedded partitioner over all 1,277 examples, and force-full hosted execution remains required. Thread replied to and resolved.

Optional

The review's comments tying the fixed shard topology and exact-checkout recomputation to the implementation were added before the final-candidate gate.

Skipped

Two Claude general review summaries duplicated the five inline items above; no separate action.
CodeRabbit's draft-status comment contained no review finding; no action.

No new follow-up issue was created. Issue #4683 already owns hosted timing and post-merge replay evidence.

Current-head independent Codex review at 1f5c104d9 found no actionable correctness regressions.

Next default scan starts after this comment. Say check all reviews to rescan the full PR.

claude · 2026-07-18T13:36:36Z

Review: split generator specs into setup-safe subshards

Overview

Replaces the single generators CI shard with three deterministic subshards (generators-1/2/3), each built at job-start time by running a --dry-run over spec/react_on_rails/generators, walking the RSpec example-group tree to find "atomic units" (groups that own a before(:context)/after(:context) hook, which can't be split across shards without repeating expensive setup), then greedily bin-packing those units across 3 shards by weight (example count + setup-hook count). The matrix and unit-shard exclusion logic is otherwise unchanged, and hosted-ci-safety.test.cjs is updated to match the new 3-way shard list.

Strengths

Fail-closed design: aborts loudly on an empty manifest, duplicate scoped IDs, or a lost/duplicated example after partitioning - a good defensive posture for a script that only runs on hosted CI and is easy to get subtly wrong.
No cross-run state: the manifest is rebuilt from the exact checked-out head in every job, avoiding the staleness/cache-invalidation problems a persisted timing database or leaf-hash cache would introduce (the PR body notes an earlier leaf-hash approach was rejected for exactly this reason).
Matrix contract tests are real: runGemMatrix actually executes the bash/jq matrix-generation script and asserts on its output, not just pattern-matching the source.

Concerns

Duplicated magic number (shard_count = 3) - hardcoded in the run script and implicitly assumed by two separate include arrays in the matrix step. Nothing ties these together; shrinking the matrix without updating the run script's constant would silently drop the highest-indexed shard's examples (no job would ever execute them, and nothing aborts, since the partitioner would just never be asked to produce a removed shard). See inline comment.
Reliance on a private rspec-core API (hooks.send(:all_hooks_for, position, :context)) to detect context-scoped hooks. The comment acknowledges this is an unstable seam; a future rspec-core bump could break every generator job with a NoMethodError buried inside a bundle exec ruby -e one-liner, which will be a rough debugging experience for whoever hits it. Worth pinning/documenting the expected failure mode. See inline comment.
Test coverage gap for the hard part: hosted-ci-safety.test.cjs only regex-matches the text of the embedded Ruby script (e.g., checking that Digest::SHA256.hexdigest(unit_id) appears somewhere) - it never actually executes the partitioning logic against a fixture spec suite. The riskiest code here (group-tree traversal/merging, greedy bin-packing, private-API hook detection) has zero executable test coverage, only string-shape assertions. Given the project's existing convention of extracting CI logic into script/* files with dedicated tests, this would benefit from the same treatment. See inline comment.
Embedding ~100 lines of Ruby as a single-quoted heredoc string inside YAML makes it invisible to normal tooling (RuboCop, editor syntax highlighting, ruby -c on its own) and harder to review/diff cleanly going forward - another reason to prefer extracting it to a real .rb file.

Minor

The sink = File.open(File::NULL, "w") handle used to silence the dry-run's own output is never explicitly closed; trivial in a short-lived one-off process, but worth a block form if this script grows.

Performance/cost note

The dry-run (which loads every generator spec file to build the group tree) is now paid 3x - once per subshard job - instead of once. Since these are parallel jobs this doesn't add to wall-clock critical path, but it does add total runner-minutes. The PR body is upfront that actual hosted timing/variance is still UNKNOWN pending a force-full run, which seems like the right way to validate this before leaning on it.

Security

No user-controlled input reaches the new script (matrix values are static enumerations in the same file); no secrets or cache reuse introduced. No concerns here.

justin808 · 2026-07-18T13:37:11Z

Address-review cutoff: `27d81b4beadad20b74301f17ee53c7f2000ad773`

Fresh review comments through 2026-07-18 13:35:06 UTC were triaged against the exact head.

MUST-FIX: 0
DISCUSS: 3, all answered and resolved with evidence
OPTIONAL: 0
SKIPPED: 0
Unresolved review threads after this pass: 0

Dispositions:

The three-row generator topology is enforced by an executable matrix-contract test for both optimized and full branches; removing or adding a row fails the contract or aborts.
The private RSpec seam is intentionally documented and fail-loud. Local exact-head execution verified its current signature; force-full hosted CI must exercise latest and minimum dependency lanes before merge.
Extracting a new script/fixture suite was declined as out-of-scope restructuring without a demonstrated residual defect. Independent exact-head execution generated and dry-ran the exhaustive 1,277-example union at 426/425/426.

No code change was required by this review pass. Any later comment or commit invalidates this cutoff and requires another pass.

justin808 · 2026-07-18T13:40:53Z

+ci-status

github-actions · 2026-07-18T13:41:05Z

CI Status

Head SHA: 27d81b4beada
Changed files: 2
Docs-only heuristic (matches ci-changes-detector metadata paths): no
ready-for-hosted-ci label: absent
force-full-hosted-ci label: absent
Current hosted-CI waiver: not present for this SHA
Automatic release-target hosted mode: inactive
Observed exact-head coverage: modes[missing=9]; successful=0, pending=0, failed=0, missing=9

Only the required gate is active unless hosted CI is requested.

justin808 · 2026-07-18T13:41:16Z

+ci-force-full

github-actions · 2026-07-18T13:41:44Z

Force-Full Hosted CI Requested

Triggered 9 workflow(s) for 27d81b4beada.
Skipped 0 workflow(s) with equivalent exact-head coverage.
Mode: force-full hosted CI (bypasses optimized change selection).
Added ready-for-hosted-ci and force-full-hosted-ci, so future commits will bypass optimized hosted CI selection until +ci-stop-full is used.

View progress in the Actions tab.

greptile-apps · 2026-07-18T13:42:57Z

Greptile Summary

This PR splits generator specs into deterministic, setup-safe CI subshards. The main changes are:

Three generator shards for each selected dependency level.
Head-local manifests built from stable RSpec scoped IDs.
Atomic grouping for examples that share context hooks.
Validation for empty, duplicate, lost, or repeated selections.
Safety tests for the expanded matrix and partitioning rules.

Confidence Score: 5/5

This looks safe to merge.

No blocking issues found in the changed code.
Invalid or incomplete partitions stop the job instead of silently reducing coverage.
The matrix retains unit coverage and both dependency levels in full runs.

Important Files Changed

Filename	Overview
.github/workflows/gem-tests.yml	Adds three deterministic generator subshards with setup-safe grouping, partition checks, and scoped-ID execution.
.github/workflows/hosted-ci-safety.test.cjs	Updates matrix expectations and adds checks for the generator-sharding invariants.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Choose optimized or full matrix] --> B{Shard type}
    B -->|unit| C[Run non-generator specs]
    B -->|generator| D[Dry-run generator specs]
    D --> E[Collect scoped IDs]
    E --> F[Build setup-safe atomic units]
    F --> G[Balance units across three shards]
    G --> H[Validate the partition]
    H --> I[Run selected examples]

%%{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
    A[Choose optimized or full matrix] --> B{Shard type}
    B -->|unit| C[Run non-generator specs]
    B -->|generator| D[Dry-run generator specs]
    D --> E[Collect scoped IDs]
    E --> F[Build setup-safe atomic units]
    F --> G[Balance units across three shards]
    G --> H[Validate the partition]
    H --> I[Run selected examples]

_{Reviews (1): Last reviewed commit: "Simplify generator shard bookkeeping" | Re-trigger Greptile}

justin808 · 2026-07-18T13:45:18Z

Address-review delta cutoff: `27d81b4beadad20b74301f17ee53c7f2000ad773`

Ready-for-review feedback through 2026-07-18 13:44:11 UTC was triaged against the exact head.

MUST-FIX: 0
DISCUSS: 3, all answered and resolved
OPTIONAL: 0
SKIPPED: 0
Unresolved review threads after this pass: 0

Dispositions:

The fixed three-shard topology remains protected by the executable optimized/full matrix contract; schema extraction was declined without a present defect.
Preserving dry-run error output is a valid diagnostic refinement, but it was declined after the final-candidate debounce point because it would invalidate nine running exact-head force-full workflows for an error-reporting-only change. Issue Follow-up: Shorten gem generator CI critical path #4683 remains the replay/fix owner if an opaque failure is observed.
Standalone fixture extraction was declined as repeated scope expansion. Real exact-head execution already proved the exhaustive partition and the force-full run is exercising all six generator jobs.

No code change was required. Any later review comment or commit invalidates this cutoff.

claude · 2026-07-18T13:45:28Z

Review: CI split generator specs into setup-safe subshards

Overview
Splits the generators hosted-CI matrix shard into three (generators-1/2/3), each dependency level. Instead of a static split, an inline Ruby script (embedded in the Run rspec tests step) runs RSpec::Core::Runner.run(..., "--dry-run", ...) to enumerate all generator examples, groups examples that share a before(:context)/after(:context) hook into "atomic units" (so shared, expensive setup is never split across shards), then greedily bin-packs those units across shards by weight (example count + hook count) using a SHA256-based deterministic tie-break. hosted-ci-safety.test.cjs is updated to assert the new matrix shape and to pattern-match several invariants in the embedded script's source.

What looks solid

The atomic-unit merge logic (walking the example-group tree top-down, unioning descendants under whichever unit they're already assigned to) correctly keeps nested before(:context) setup together, and aborts loudly if a nested group's descendants would need to span two different units, a reasonable fail-closed guard.
Partition validation (assigned_ids.sort == ids.sort, empty/duplicate-ID checks, empty-shard check) gives good fail-closed behavior for the internal partition step.
No caching, secrets, or cross-head state is introduced, each job rebuilds its manifest from its own checkout, avoiding stale-partition risk.
The matrix/test changes in hosted-ci-safety.test.cjs are consistent with the file's existing conventions (extractRunScript + assertMatches/assertDoesNotMatch).

Issues raised inline

Duplicated shard-count source of truth (gem-tests.yml): the shard_count=3 literal in the run step and the generators-1/2/3 rows in the two matrix jq blocks can drift independently, a mismatch (fewer matrix rows than shard_count) would silently drop coverage rather than fail loudly, since the internal partition-completeness assertion only validates within the script's own shard_count, not against what was actually dispatched.
Swallowed dry-run failure output: RSpec::Core::Runner.run stdout/stderr is piped to File::NULL, so a real spec-loading error (syntax/load/name error in a generator spec) surfaces in CI only as a generic "dry run failed with status N" with no stack trace, making failures hard to diagnose.
Test coverage is source-text pattern matching, not behavioral: the new hosted-ci-safety.test.cjs assertions confirm certain substrings exist in the embedded Ruby script but never execute the partitioning algorithm against fixture data, so the actual balancing/atomic-grouping/tie-break behavior (the interesting part of this PR) is untested by anything that would catch a real logic regression.

Other notes (not blocking, FYI)

The script relies on a private RSpec API (Hooks#send(:all_hooks_for, ...)) since there's no public hook-enumeration method; this is called out in a code comment and fails loudly (abort) if it breaks, a reasonable mitigation, but still a private-API dependency worth watching across rspec-core version bumps.
Embedding ~90 lines of non-trivial Ruby directly in a YAML run: block (rather than a checked-in .rb script) makes it harder to lint/type-check/unit-test directly and harder to read/diff; worth considering extraction to a script file if this logic grows further.
Each of the 3 generator shard jobs independently re-runs a full --dry-run over all generator specs to rebuild the complete manifest before filtering to its own third, likely cheap since dry-run skips hook/example execution, but flagging as a 3x redundant "list all examples" cost per full-matrix run.

No security concerns (no secrets/permissions touched) and no functional application code is affected, this is CI-only.

justin808 · 2026-07-18T14:12:44Z

+ci-status

github-actions · 2026-07-18T14:12:56Z

CI Status

Head SHA: 27d81b4beada
Changed files: 2
Docs-only heuristic (matches ci-changes-detector metadata paths): no
ready-for-hosted-ci label: present
force-full-hosted-ci label: present
Current hosted-CI waiver: not present for this SHA
Automatic release-target hosted mode: inactive
Observed exact-head coverage: modes[force-full=9]; successful=9, pending=0, failed=0, missing=0

Force-full hosted CI is enabled for this PR.

justin808 · 2026-07-18T17:02:39Z

Independent batch QA — READY

Exact head: 27d81b4beadad20b74301f17ee53c7f2000ad773. CI READY, zero unresolved threads, strict merge ledger complete, deterministic setup-safe 1,277-example sharding and repeated hosted benchmark evidence satisfied. Issue #4683 contains the exact post-merge secondary-event exercise and cleanup contract.

CI: shorten generator critical path with subshards

edb9f7c