Conversation
github-actions
Bot
added
the
maintainer:e2e:ok
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR partitions CLI test files into three deterministic subsets. CI expands CLI shards into three matrix rows and passes each partition to the Bun runner. Reports and artifacts include partition identifiers. ChangesCLI test partitioning
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
WalkthroughBefore this change, the CLI test shard ran as a single CI job, so every CLI test file executed in one workflow run. That serialized the entire CLI suite behind one runner slot and made the shard a wall-clock bottleneck in PR validation. After this PR, the affected-test-shards selector emits three matrix entries for the CLI shard, and the Bun test runner filters its discovered files by LLXPRT_CLI_TEST_PARTITION using deterministic round-robin assignment. Each partition now runs a fixed subset of the CLI test files in parallel, cutting the shard’s critical-path time while preserving full coverage across the three runs. Release NotesNew Features
Tests
Documentation
Changes
Magnitude🎯 1 (S) Related
Pre-merge Checks
Walkthrough generated by LLxprt PR Review. Planner issue: #2256 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
scripts/affected-test-shards.ts (1)
862-874: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a prototype-safe lookup for the partition count.
SHARD_PARTITION_COUNTS[shard]resolves inheritedObject.prototypekeys. A shard namedconstructorortoStringwould return a function, andArray.from({ length: fn })yields zero rows, which drops the shard from the matrix silently. Shard names are internal today, so this is defensive only.♻️ Proposed prototype-safe lookup
return selectedShards.flatMap((shard) => { - const count = SHARD_PARTITION_COUNTS[shard] ?? 1; + const count = Object.hasOwn(SHARD_PARTITION_COUNTS, shard) + ? SHARD_PARTITION_COUNTS[shard] + : 1; return Array.from({ length: count }, (_, i) => ({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/affected-test-shards.ts` around lines 862 - 874, Update buildMatrix’s partition-count lookup to accept only own properties of SHARD_PARTITION_COUNTS, falling back to 1 for inherited or absent shard names. Preserve the existing matrix row generation and partition labels for valid configured counts..github/workflows/ci.yml (1)
1091-1096: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider running the CLI smoke test in only one partition leg.
The smoke step condition stays
matrix.shard == 'cli', so the step now runs three times instead of once. The step output is identical in every leg. The same applies tonpm run lint:test-shardsat Line 1041, which is partition-independent.Issue
#3185targets the critical path, so redundant fixed cost per leg works against the goal. Gate these steps on a single partition if you want the saving.♻️ Proposed condition change for the smoke step
- if: matrix.shard == 'cli' + if: matrix.shard == 'cli' && matrix.partition == '1of3'If you prefer to keep per-leg redundancy as a safety net, no change is needed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 1091 - 1096, Restrict the CLI smoke test step identified by “Smoke test CLI entry” to a single CLI matrix partition instead of every leg, while preserving the CLI-only requirement; apply the same single-partition gating to the partition-independent npm run lint:test-shards step.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 1091-1096: Restrict the CLI smoke test step identified by “Smoke
test CLI entry” to a single CLI matrix partition instead of every leg, while
preserving the CLI-only requirement; apply the same single-partition gating to
the partition-independent npm run lint:test-shards step.
In `@scripts/affected-test-shards.ts`:
- Around line 862-874: Update buildMatrix’s partition-count lookup to accept
only own properties of SHARD_PARTITION_COUNTS, falling back to 1 for inherited
or absent shard names. Preserve the existing matrix row generation and partition
labels for valid configured counts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0341ef4d-c9d0-45be-8f39-8d95631c3d73
⛔ Files ignored due to path filters (1)
project-plans/issue3185/plan.mdis excluded by!project-plans/**
📒 Files selected for processing (7)
.github/workflows/ci.ymlpackages/cli/run-bun-tests.tspackages/cli/test/run-bun-tests.test.tsscripts/affected-test-shards.tsscripts/tests/affected-test-shards-partition.test.tsscripts/tests/affected-test-shards.test.tsscripts/tests/ci-test-shard-partition.test.ts
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
OpenCodeReview — automatic reviews suspendedAutomatic OCR reviews are suspended for this PR after 2 of 2 automatic reviews. To get more reviews you can:
OpenCodeReview — PR #3197
|
|
Addressed the OCR matrix-test finding in commit b1ac552. The non-CLI case now asserts that buildMatrix(['core']) returns exactly one row before verifying its 1of1 partition identity, so duplicate-row regressions cannot pass unnoticed. Focused test: 7 passed, 0 failed; ESLint, the ESLint policy guard, and formatting also passed. |
Candidate CI evidence — target not metAuthoritative runs: https://github.com/vybestack/llxprt-code/actions/runs/31306562738, attempts 1–3, all successful on implementation SHA 102bb71.
This does not satisfy the required mean below 600 seconds. The CLI split itself is successful and stable:
Each attempt executed and passed all 682 files as 228/227/227. Each attempt reported the same aggregate CI case inventory: 8,814 passed, 0 failed, 27 pre-existing platform-conditional skipped, and 13 todo (8,854 total). All three partition-specific JUnit publish steps passed with fail-on-empty enabled and distinct reporter names. Local test-file coverage found zero uncovered and zero doubly executed files; the six logical shard definitions still cover all 16 workspaces. The critical path moved outside CLI. Attempt 2's scripts job took 551s, including 444s in Run shard tests. Attempt 3's scripts job took 572s, while Javascript lint also completed after the 600-second boundary. After the test-only OCR remediation, final SHA b1ac55290871baf56952a4ed6050571fb3c2eb6b had another fully green run, https://github.com/vybestack/llxprt-code/actions/runs/31308353655: CLI jobs were 256–293s, but scripts took 580s and the required Test verdict completed 689s after workflow creation. All current PR checks are green and the OCR finding was addressed and resolved. The accepted timing criterion remains incomplete. Partitioning or optimizing the unrelated scripts shard and/or Javascript lint would expand the agreed CLI-only scope, so no such change has been made without user direction. |
1 participant
TLDR
Splits the existing logical CLI test shard into three deterministic physical GitHub Actions matrix legs so CLI-affected pull requests no longer wait on one roughly 12-minute test job.
All 682 currently discovered CLI test files remain structurally discovered before partitioning. Sorted round-robin assignment produces inventories of 228, 227, and 227 files, while unpartitioned local and nightly execution remains unchanged.
Dive Deeper
The implementation plan and evidence ledger are in project-plans/issue3185/plan.md. Authoritative candidate-head wall-clock evidence will be appended after at least three successful GitHub Actions runs on this identical implementation.
Reviewer Test Plan
Run the focused behavior suite:
Verify complete and unique test-file ownership:
Exercise each physical CLI partition:
Confirm an unpartitioned CLI run still executes the full inventory:
Inspect the Actions matrix for three distinct CLI jobs and distinct JUnit reporter names. Confirm the required Test aggregator waits for every selected matrix leg.
Local evidence:
Full-suite local caveat: repeated exact npm run test executions completed all workspaces but returned failure because different packages/agents child processes hit the existing 180-second watchdog under local macOS process contention. Every reported agents file passed promptly when rerun alone, no agents file is changed here, and the CLI suite itself completed 682/682 files with the case totals above. Candidate-head GitHub Actions remains the required merge gate and authoritative performance environment.
Testing Matrix
Linked issues / bugs
Fixes #3185
Part of #2702
Summary by CodeRabbit
Performance
Bug Fixes
Testing