Skip to content

improve(ci): shorten compact PR test critical path#104508

Merged
obviyus merged 2 commits into
mainfrom
codex/pr-ci-compact-balance
Jul 11, 2026
Merged

improve(ci): shorten compact PR test critical path#104508
obviyus merged 2 commits into
mainfrom
codex/pr-ci-compact-balance

Conversation

@obviyus

@obviyus obviyus commented Jul 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

Code pull requests repeatedly waited on checks-node-compact-large-whole-1, which serialized six fixed Vitest groups. Across six recent successful PR runs, that job took 292-318 seconds and consistently owned the critical path.

Why This Change Was Made

When a runner/dist bucket would create one concentrated whole-config job, spread those groups evenly across compatible compact jobs that are already admitted and still below the group cap. Destinations that absorb whole-config groups retain the existing 120-minute timeout introduced in #95681. This reduces the canonical PR Node matrix from 20 jobs to 19 while leaving main, manual, and release topology unchanged.

User Impact

Contributors get faster code-PR CI without adding runner pressure. Two exact-head runs observed savings of 10-64 seconds (3-19%). Product runtime behavior is unchanged.

AI-assisted: yes. I understand the planner change and reviewed the resulting job topology.

Evidence

Metric Before (reference) After (run 1) After (run 2)
Full PR CI wall time 343s 279s (-64s, -19%) 333s (-10s, -3%)
Compact critical job 309s (dedicated whole job) 223s (-86s, -28%) 303s (-6s, -2%)
Node test jobs 20 19 19
  • Historical contract: fix(ci): debounce canonical main runner admission #95681 introduced bounded compact whole-suite batches with a 120-minute timeout; improve(ci): balance compact whole test jobs #104472 preserved that timeout while rebalancing the dedicated jobs. Every mixed destination in this change retains it.
  • Registration budget: Node test jobs decrease from 20 to 19; the live organization bucket was 10,000 registrations per five minutes with 10,000 remaining during validation.
  • Blacksmith Testbox tbx_01kx8s3ktsdv2ts3bva6fc3148 (blacksmith-testbox), stopped after proof:
    • corepack pnpm test test/scripts/ci-node-test-plan.test.ts — 19 tests passed
    • pnpm check:changed — passed (testRoot, docs, tooling)
  • Autoreview (gpt-5.6-sol) — clean, no actionable findings

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation scripts Repository scripts size: XS maintainer Maintainer-authored PR labels Jul 11, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jul 11, 2026
@clawsweeper

clawsweeper Bot commented Jul 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 11, 2026, 10:45 AM ET / 14:45 UTC.

Summary
The PR spreads a lone batch of whole-config compact tests across compatible existing PR jobs, preserves their extended timeout, updates focused planner coverage, and documents the resulting 19-job topology.

PR surface: Tests +9, Docs 0, Other +20. Total +29 across 3 files.

Reproducibility: not applicable. as a product bug; the CI bottleneck and improvement are directly observable from deterministic planner output and successful exact-head Actions runs.

Review metrics: 2 noteworthy metrics.

  • Compact Node matrix: 1 job removed; 20 to 19. The optimization reduces Blacksmith registrations while retaining the planned test-group coverage.
  • Observed PR wall time: 10–64 seconds faster; 3–19%. Two successful exact-head runs demonstrate a measurable contributor feedback improvement against the stated reference run.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

  • [P1] The patch changes canonical pull-request test admission, so any refreshed head should rerun exact-head required checks before landing.
  • [P1] The PR carries the protected maintainer label and therefore requires explicit maintainer handling even though the patch review is clean.

Maintainer options:

  1. Land with exact-head gating (recommended)
    Merge after protected-label review and successful required checks on the exact head, preserving the planner invariants covered by the patch and tests.
  2. Pause if topology drifts
    Hold the PR if a refreshed head changes the planner surface or no longer proves complete group coverage and timeout propagation.

Next step before merge

  • No automated repair is needed; proceed through normal protected-label maintainer review and exact-head check gating.

Security
Cleared: The patch changes deterministic CI planning, tests, and documentation without adding dependencies, permissions, secrets, downloads, package-resolution changes, or third-party execution.

Review details

Best possible solution:

Merge the focused optimization after protected-label review and exact-head gating, retaining the single canonical planner path and its coverage, runner/dist, group-cap, subprocess-isolation, and timeout invariants.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a product bug; the CI bottleneck and improvement are directly observable from deterministic planner output and successful exact-head Actions runs.

Is this the best way to solve the issue?

Yes. The patch modifies the existing canonical planner, reuses compatible admitted jobs, reduces registrations, preserves the whole-config timeout, and avoids creating a parallel CI execution path.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 72aa3f1bcf4c.

Label changes

Label changes:

  • add merge-risk: 🚨 automation: The patch changes canonical PR test packing, where an invariant regression could affect coverage, timeout handling, or CI availability after merge.
  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

  • P3: This is a focused internal CI latency improvement with no product runtime behavior change.
  • merge-risk: 🚨 automation: The patch changes canonical PR test packing, where an invariant regression could affect coverage, timeout handling, or CI availability after merge.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): Two linked successful exact-head Actions runs demonstrate the after-change topology and wall-time improvement, supplemented by focused Blacksmith Testbox validation.
  • proof: sufficient: Contributor real behavior proof is sufficient. Two linked successful exact-head Actions runs demonstrate the after-change topology and wall-time improvement, supplemented by focused Blacksmith Testbox validation.
Evidence reviewed

PR surface:

Tests +9, Docs 0, Other +20. Total +29 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 1 10 1 +9
Docs 1 1 1 0
Config 0 0 0 0
Generated 0 0 0 0
Other 1 21 1 +20
Total 3 32 3 +29

What I checked:

  • Compatible planner boundary: Before packing, the compact planner partitions every test group by resolved runner and dist requirement, so redistributed whole-config groups remain within an existing compatible execution bucket. (scripts/lib/ci-node-test-plan.mjs:1153, d914e378e2b2)
  • Bounded redistribution and timeout: Redistribution is limited to one whole batch, multiple destinations, and bins that remain within the eight-group cap; every destination receiving a whole-config group receives the existing 120-minute timeout. (scripts/lib/ci-node-test-plan.mjs:1195, d914e378e2b2)
  • Focused regression coverage: The updated test verifies the 19-job plan, runner/dist consistency, complete include-pattern coverage, 120-minute timeout for every whole-config-bearing job, an even 2/2/2 large-job distribution, and removal of the dedicated large-whole job. (test/scripts/ci-node-test-plan.test.ts:176, d914e378e2b2)
  • Workflow contract: The CI manifest consumes the planner's groups, runner, dist requirement, environment, and timeout fields directly; the job timeout therefore follows the redistributed whole-config groups. (.github/workflows/ci.yml:421, 72aa3f1bcf4c)
  • Current-main comparison: Current main has no intervening changes to the planner, its focused test, CI documentation, or workflow consumer since the PR base, so the behind state does not introduce a known touched-surface conflict. (scripts/lib/ci-node-test-plan.mjs:1148, 72aa3f1bcf4c)
  • Exact-head real behavior proof: The PR supplies two successful exact-head Actions runs showing the replacement topology and 279- and 333-second completion times against a 343-second reference, plus a focused 19-test Blacksmith Testbox run and changed-surface checks. (d914e378e2b2)

Likely related people:

  • vincentkoc: Introduced the merged whole-group balancing and related timeout/admission behavior directly extended by this PR. (role: recent CI planner contributor; confidence: high; commits: b4c6ee21e30c, 607b2e96634c; files: scripts/lib/ci-node-test-plan.mjs, test/scripts/ci-node-test-plan.test.ts, docs/ci.md)
  • steipete: Git history attributes the canonical compact binning, group caps, and final job construction to Peter Steinberger's planner work. (role: planner introducer and recent area contributor; confidence: high; commits: 3902adf9f673; files: scripts/lib/ci-node-test-plan.mjs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (3 earlier review cycles)
  • reviewed 2026-07-11T14:20:46.407Z sha 70aea2e :: needs maintainer review before merge. :: none
  • reviewed 2026-07-11T14:33:17.004Z sha d914e37 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-11T14:39:13.740Z sha d914e37 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 11, 2026
@obviyus obviyus self-assigned this Jul 11, 2026
@obviyus
obviyus merged commit f506070 into main Jul 11, 2026
141 of 153 checks passed
@obviyus
obviyus deleted the codex/pr-ci-compact-balance branch July 11, 2026 15:03
@obviyus

obviyus commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

@github-actions github-actions Bot mentioned this pull request Jul 11, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 12, 2026
@obviyus
improve(ci): shorten compact PR test critical path (openclaw#104508)
8f58a14 
* perf(ci): rebalance compact PR test jobs

* fix(ci): preserve compact whole-job timeout
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@obviyus obviyus

Labels

docs Improvements or additions to documentation maintainer Maintainer-authored PR merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. scripts Repository scripts size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@obviyus