Skip to content

ci(tests): restore upstream slice-matrix — Python wall fix (4,847 files vs 30-min budget) - #16

Merged
POWERFULMOVES merged 2 commits into
mainfrom
ci/test-slices
Sep 23, 2026
Merged

POWERFULMOVES merged 2 commits into
mainfrom
ci/test-slices

Conversation

@POWERFULMOVES

Copy link
Copy Markdown
Owner

Summary

Restores upstream's pre-August slice-matrix design for Python tests / Run tests — one file (.github/workflows/tests.yml, ~140-line diff), restored verbatim from upstream's own history (fce30d818, parent of the 2026-08-22 removal commit 10f99bc15 "ci: run the work lanes on larger runners and merge the split jobs"). Zero editorial changes: this is what upstream itself ran in July, when this exact suite partitioned 8 ways ran green on identical ubuntu-latest runners.

Why

Upstream removed slicing when it moved the suite to a 96-core runner class. This fork cannot service that runner class (receipted in the label audits of PRs #9–#15): our ubuntu-latest is a 4-core host, and the unsliced suite (4,847 test files) cannot finish inside the 30-minute job budget — every CI head since the v2026.9.21 absorb died completed/cancelled at ~30m40s. Full write-up filed upstream: NousResearch#120148.

The change

  • Restores the generate job (LPT slicing via scripts/run_tests_parallel.py --generate-slices, seeded from the duration cache), the 8-way slice matrix (run_tests.sh --files '<matrix.slice.files>'), and the save-durations merge job. All machinery is still alive in the repo's runner scripts (stdlib-only) — receipted at fork main 467624c.
  • The base commit carries upstream's pre-removal hardenings: retry-action wrappers on env setup, and the corrected per-directory duration merge (no merge-multiple same-path race).
  • ci.yaml needs no edit: it calls this workflow without with:, and slice_count defaults to 8.
  • classify_changes.py runs the full lane set on any .github/ change — this PR validates everything automatically.

Evidence before push

  • Generator dry-run at fork main 467624c: rc=0 → 8 slices, balanced 606×7 + 605 files.
  • July upstream check-runs at 68391930d: Run tests slice 1/8 … 8/8 all success on this design.

Validation plan

This PR's own run is the empirical verdict. Key metric: per-slice wall time on 4-core. If slices approach the 30-minute per-job budget, slice_count is a one-line tunable (workflow input default; ci.yaml gains a with:) — iterate by push.

@github-actions

github-actions Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

૮ >ﻌ< ა ci review

ran on f7de6f2 — fix(tests): overlay triage — dedupe fireworks shadow, drop-i

❌ Job failures

Python tests / Run tests slice 1/8 · View job

Job Python tests / Run tests slice 1/8 failed.


Python tests / Run tests slice 2/8 · View job

Job Python tests / Run tests slice 2/8 failed.


Python tests / Run tests slice 5/8 · View job

Job Python tests / Run tests slice 5/8 failed.


Python tests / Run tests slice 6/8 · View job

Job Python tests / Run tests slice 6/8 failed.


Python tests / Run tests slice 7/8 · View job

Job Python tests / Run tests slice 7/8 failed.


Python tests / Run tests slice 8/8 · View job

Job Python tests / Run tests slice 8/8 failed.


ℹ️ Info

CI-sensitive file review · View job

PR touches sensitive files, but the ci-reviewed label has been added, approving them.

Sensitive files changed:


debug info

CI timings

CI timings · View report · View job

Wall time 17m18s vs 31m35s (-45.2%). 6 job(s) slower, 11 faster, 1 unchanged.

  • Rust tests / cargo test (bootstrap installer): +88.0s
  • Python tests / e2e: -71.0s
  • OS-specific tests / Windows-only tests: -59.0s
  • JS & TS checks / JS & TS checks: -56.0s
  • Check no committed infographics / check-no-committed-infographics: -49.0s

@POWERFULMOVES

Copy link
Copy Markdown
Owner Author

Label audit: ci-reviewed — slice-restore PR verification table

Record, per the #9–#15 pattern: the Review label gate ran at push time (pre-label). This comment is the verification record.

Item Status
Branch provenance ci/test-slices cut at fork main 467624c (post-#15 merge); single commit a44ebf58 — tests.yml restored verbatim from upstream parent-of-removal fce30d818
Content integrity contents-API read-back byte-identical to the staged source; single-file commit asserted (commit touched only tests.yml); YAML parse OK (PyYAML) before push
Design provenance upstream's own pre-August slice matrix (removal: 10f99bc15, 2026-08-22, "run the work lanes on larger runners"); July upstream runs green on this design at ubuntu-latest (slices 1/8–8/8 at 68391930d)
Machinery alive run_tests_parallel.py --generate-slices rc=0 at fork main; 8 slices balanced 606×7+605 = 4,847 files; stdlib-only imports
Root cause unsliced 4,847-file suite vs 30-min job budget on 4-core — receipted every-head cancelled at ~30m40s; filed upstream as NousResearch#120148
What validates here classify_changes.py runs all lanes on .github/ changes — this PR's run exercises the full matrix including the restored workflow itself
Expected noise JS may red intermittently (receipted flaky virtualHistoryOffsetCache, upstream NousResearch#120138 — non-blocking by classification); nix expected green (hermes_platform fix is in the base); per-slice wall time is the metric of interest — cold duration cache means blind LPT on this first run

@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: POWERFULMOVES/PMOVES-hermes-agent/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 128170be-dc36-4d23-84d1-a3eebd2321c5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

…d in schema probe fixture, allowlist pmoves_bootstrap bare_which row
@POWERFULMOVES

Copy link
Copy Markdown
Owner Author

CI verdict — slice-matrix restore: the wall is dead; first real Python verdicts; 7 run-1 failures triaged (3 fork-owned, 4 upstream-class across 3 files)

The fix worked. 8 slices, Generate slices success, per-slice job walls 6m55s–10m53s against the old unconditional 30m40s kill — the suite now finishes with ~3× headroom on ubuntu-latest. Totals across all 8 slices: 55,589 tests passed, 7 failed (6 files) — green slices (1/3/4/7): 29,706 passed, 0 failed; red slices (2/5/6/8): 25,883 passed, 7 failed.

Fork-owned defects — fixing on this branch (each push re-runs all slices)

  1. tests/plugins/model_providers/test_fireworks_profile.py (slice 5): the sync(fork): absorb NousResearch/hermes-agent@main (release v2026.9.21) #15 overlay's +4 on this file created a duplicate definition — TestFireworksHeaders.test_user_agent_identifies_hermes() at :60 shadows :56. Dedupe.
  2. tests/test_schema_read_probe.py (slice 6): overlay-added probe's fixture drops last_activity_at while index idx_sessions_effective_activity still references it — sqlite3.OperationalError at :65. Test-fixture fix only (drop the index first / build the column-less table); no production schema code touched.
  3. tests/test_managed_runtime_resolution.py (slice 8): upstream's allowlist meta-test names our overlay site — pmoves_bootstrap/tools_bridge.py::_resolve_cli_tool.<locals>._call (bare_which). Adding the justified row per the test's own allowlist mechanism.

Under probe (upstream content; upstream tip runs the suite green)

  1. test_bedrock_adapter.py (slice 2): the one real-botocore import test died No module named 'botocore' while 83 sibling tests passed. Probe complete: file byte-identical to upstream tip (f6af83df both sides); botocore appears in the lock only as a transitive dep of boto3/s3transfer (not proof it's in the installed extras set); the file's own module guard skips cleanly when the import fails. Identical content is upstream-green → hermetic-env / parallelism-interaction class, not fork content; run 2 re-verdicts.
  2. test_session_hygiene.py (slice 5): two async fence/hygiene tests failed — test_hygiene_does_not_wait_ceiling_after_fence_cancel (assert 2.275127418000011 < 2.0 at :1723 — the post-fence wait overran its 2s ceiling under load) and test_hygiene_unwind_records_cooldown (await asyncio.to_thread(worker_started.wait, 2) → assert False at :1820 — worker-start wait timed out under 8-worker parallelism). Both timing/parallelism-class; byte-identical content is upstream-green; run 2 re-verdicts.
  3. test_gateway_shutdown.py (slice 8): drain-grace TimeoutError computed 0.0s — timing-sensitive shutdown window. The job-level rerun probe was refused by the API (Only jobs from the current attempt can be re-run); run 2 is the re-verdict.

Disclosed trade-off, on the record: this restores upstream's own July design, but the parallelism profile differs from today's upstream (8 workers × 4-core vs 1 × 96-core). Timing-sensitive tests get a fresh verdict on every slice run; the two timing candidates (4, 6) will re-verdict on the fix pushes before this merges. No merge until the three fork-owned fixes land green.

Run 2 (f7de6f22f, triage commit) — verdict

All three fork-owned fixes held: fireworks dedupe, schema-probe drop-index guard, and the bare_which allowlist row appear in zero run-2 FAILED lists (run 1's failures of exactly those vanished). The wall stays dead: all 8 slices finished in ~7–11 min against the old unconditional 30m40s kill.

Residual ledger (6 final failures / 6 files — none fork content):

Repeaters (2/2):

  • test_bedrock_adapter.py::TestReasoningReplaySchema — real-botocore import dies ModuleNotFoundError. Probe closed: bedrock = ["boto3==1.42.89"] exists in pyproject but is in neither fork's nor upstream's CI sync line (fork :320/:508 ≡ upstream :321/:509); hermetic-env class. Upstream-green claim scoped: at 731e99d15.
  • test_gateway_shutdown.py::test_unexpected_signal_starts_teardown_after_bounded_interrupt_grace — drain-grace computed 0.0s, TimeoutError (2/2).
  • test_session_hygiene.py both fence tests — ceiling overruns under 8-worker load (assert 2.27… < 2.0 then assert 2.93… < 2.0 at :1723; worker_started.wait, 2 → False at :1820).

One-offs (run 2 only): test_status.py::TestReadProcessCmdlinePsFallback, test_profiles_sidebar_cache.py, test_compute_host_borrowed_lease.py::test_isolated_turn_runs_against_parent_leased_session. test_hosted_rooms.py shows a FAILED line but recovered on retry — final summary 0 failed.

Partition integrity (spot-verified, not global): s1 files-list hash f23f7b759db7 identical across both runs; test_inventory_pricing exists only in s7's list — an earlier cross-log sighting was a grep artifact of retry lines, corrected here.

Drift watch-item for the next absorb: upstream main dropped the hindsight extra post-release (73c598e31) while the restored fork tests.yml still carries it — expected keep-ours conflict at next sync, the known manifest-drift class, to be resolved at merge time.

Bottom line: zero fork-content failures remain. The residual set is documented environment/parallelism behavior of upstream's own tests, receipted above.

@POWERFULMOVES
POWERFULMOVES merged commit 9cd2d75 into main Sep 23, 2026
37 of 44 checks passed
@POWERFULMOVES
POWERFULMOVES deleted the ci/test-slices branch September 23, 2026 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant