Skip to content

perf(test): parallelize local integration suite#6420

Merged
cv merged 5 commits into
mainfrom
codex/perf-local-integration-parallelism
Jul 7, 2026
Merged

perf(test): parallelize local integration suite#6420
cv merged 5 commits into
mainfrom
codex/perf-local-integration-parallelism

Conversation

@cv

@cv cv commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Run the integration project as a bounded four-worker phase during the canonical local npm test, while keeping CI, coverage, focused integration, and direct Vitest runs serialized. Isolate two onboarding fixtures from host-global dashboard ports so the parallel suite remains deterministic. This is the final cumulative #6245 step after the named onboarding conversions, representative process-contract work, and sequenced loader cleanup already merged; the final clean-build Node 22 suite passes in 3:52.03.

Related Issue

Closes #6245.

Changes

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: Test fixtures and local test-runner scheduling changed; NemoClaw commands, configuration, runtime behavior, and CI/coverage workflows are unchanged.
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Independent final-diff review confirmed that the fake lsof preserves the real CLI/preflight/exit contract, the restore-intent assertions remain intact, and resolved CI/coverage configurations remain serialized.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: Real CLI exhaustion contract passed; restore-intent passed with all 11 dashboard ports deliberately occupied; scheduling matrix passed 14/14 through the lifecycle-triggered config; npm run test:projects:check reported 1,327 files disjoint across 8 projects.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result: Clean-build Node 22 npm test -- --reporter=blob under the normal umask 022 passed 1,251 files and 13,879 tests with 39 skipped, 1 todo, and zero failures in 3:52.03, down 73% from the issue's 14:19.65 baseline despite a larger suite. The matching diff-scoped routine pre-commit stage passed in 13.95s. ci(hooks): streamline local PR verification #6270 separately removed full coverage from routine pre-commit while preserving manual and authoritative CI gates.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • New Features

    • Integration test runs now use adaptive scheduling to speed up local execution while keeping CI/focused runs serialized.
  • Bug Fixes

    • Improved reliability of onboarding regression coverage by simulating dashboard port exhaustion in a hermetic way.
    • Updated onboarding-related fixtures to better match the intended readiness/exit behavior.
  • Tests

    • Added coverage for integration scheduling behavior (local caps, invalid inputs, and CI/coverage scenarios).
  • Documentation

    • Added test-suite documentation with a local performance snapshot and key test hotspots.

@cv cv self-assigned this Jul 7, 2026
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a Vitest integration scheduling helper and wires it into the integration project config, with tests for local, CI, and argv-driven worker selection. It also makes onboarding tests hermetic by replacing real port binding with an lsof shim and updates one restore-intent fixture stub.

Changes

Integration project scheduling

Layer / File(s) Summary
Scheduling resolver implementation
test/helpers/integration-project-scheduling.ts
Defines the scheduling context, parses and clamps --maxWorkers values, and computes integration project file parallelism from CI, lifecycle event, coverage flags, and argv.
Scheduling helper tests
test/vitest-integration-scheduling.test.ts
Adds tests for local parallel scheduling, argv maxWorkers parsing and capping, invalid maxWorkers values, and serialized CI or coverage cases.
vitest.config.ts wiring
vitest.config.ts
Imports runtime parallelism and the helper, computes integrationProjectScheduling, and spreads it into the integration project test config.

Hermetic onboarding test fixes

Layer / File(s) Summary
Hermetic lsof shim
test/exit-code-user-error-surfaces.test.ts
Updates the dashboard-port exhaustion regression test comment, removes the net import, switches setup to a synchronous path, and writes a fake lsof shim that reports the dashboard ports as occupied.
Restore-intent fixture stub
test/onboard-installer-restore-intent.test.ts
Adds a forward list stub returning a running mapping in the non-ready sandbox fixture.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR addresses the core issue by parallelizing local integration runs, preserving serial CI/coverage paths, and keeping the full test signal with hermetic fixtures.
Out of Scope Changes check ✅ Passed The changes stay within test performance, scheduling, and hermetic fixture updates; nothing unrelated stands out.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: parallelizing the local integration test suite while keeping other modes serialized.
✨ 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/perf-local-integration-parallelism

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

@github-code-quality

github-code-quality Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the branch is 96%. Coverage data for the branch is not yet available.

Show a code coverage summary of the most covered files.
File 3a0ab34 +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the branch is 76%. Coverage data for the branch is not yet available.

Show a code coverage summary of the most covered files.
File 3a0ab34 +/-
src/lib/onboard/preflight.ts 82%
src/lib/actions...all/run-plan.ts 81%
src/lib/state/o...oard-session.ts 81%
src/lib/actions...licy-channel.ts 79%
src/lib/state/sandbox.ts 75%
src/lib/actions...dbox/connect.ts 72%
src/lib/onboard...er-gpu-patch.ts 69%
src/lib/policy/index.ts 66%
src/lib/shields/index.ts 61%
src/lib/onboard.ts 28%

Updated July 07, 2026 22:59 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: None
Optional E2E: None

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • None. No E2E is required. This PR is limited to test documentation, non-live test fixture changes, and Vitest scheduling for local/non-live integration tests. It does not change runtime code or behavior that can affect installer/onboarding flows, sandbox lifecycle, credentials, security boundaries, network policy, inference routing, deployment, or real assistant user flows.

Optional E2E

  • None.

New E2E recommendations

  • None.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: None
Optional E2E targets: None

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

  • None. Changes are limited to non-live local/CI Vitest integration scheduling, non-E2E tests, and test documentation. They do not modify test/e2e targets, live fixtures/support, registry/runtime wiring, e2e.yaml, or target-relevant application surfaces requiring an E2E target dispatch.

Optional E2E targets

  • None.

Relevant changed files

  • None.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Source-of-truth review needed: Forward list mock added to 'no restore intent' test case.
Open items: 0 required · 2 warnings · 7 suggestions · 5 test follow-ups
Since last review: 4 prior items resolved · 2 still apply · 3 new items found

Action checklist

  • PRA-1 Resolve or justify: Source-of-truth review needed: Forward list mock added to 'no restore intent' test case
  • PRA-2 Resolve or justify: PR fix(onboard): diagnose incomplete custom plugin images #6250 modifies the same test file — potential merge conflict or behavioral interference in test/onboard-installer-restore-intent.test.ts:270
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Unit tests cover scheduling logic but not Vitest config integration
  • PRA-T5 Add or justify test follow-up: Forward list mock added to 'no restore intent' test case
  • PRA-3 In-scope improvement: Exported resolveIntegrationProjectScheduling lacks JSDoc documentation in test/helpers/integration-project-scheduling.ts:47
  • PRA-4 In-scope improvement: Fake lsof replaces host port binding — simplification already applied in test/exit-code-user-error-surfaces.test.ts:149
  • PRA-5 In-scope improvement: Forward list mock in 'no restore intent' test may be a workaround for incidental source behavior in test/onboard-installer-restore-intent.test.ts:270
  • PRA-6 In-scope improvement: Percentage-based --maxWorkers uses Math.round which may produce unexpected clamping in test/helpers/integration-project-scheduling.ts:1
  • PRA-7 In-scope improvement: Integration project scheduling computed at config load time, fixed per run in vitest.config.ts:33
  • PRA-8 In-scope improvement: Unit tests cover scheduling logic but not Vitest config integration in test/vitest-integration-scheduling.test.ts:1
  • PRA-9 In-scope improvement: Acceptance clauses met via different design path (parallelization vs subprocess removal) in test/README.md:65

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Resolve/justify scope test/onboard-installer-restore-intent.test.ts:270 Coordinate with PR #6250 author to ensure changes are compatible. Review PR #6250's diff against this file to confirm no overlapping modifications to the same test cases.
PRA-3 Improvement docs test/helpers/integration-project-scheduling.ts:47 Add JSDoc to the exported function describing the scheduling policy: parallelizes only on local non-CI 'test' runs without coverage flags, caps workers at 4, honors --maxWorkers flag with numeric or percentage values.
PRA-4 Improvement architecture test/exit-code-user-error-surfaces.test.ts:149 No action needed — simplification already applied and verified.
PRA-5 Improvement correctness test/onboard-installer-restore-intent.test.ts:270 Verify that the forward list call is actually made in the code path under test, and that the mock return value is appropriate. If the forward list call is incidental to the not-ready decision, consider whether the test should instead verify the forward list is not called, or whether the source code should be refactored to avoid the unnecessary call.
PRA-6 Improvement correctness test/helpers/integration-project-scheduling.ts:1 Add a comment clarifying the percentage rounding behavior, and consider whether Math.ceil would be more intuitive for percentage-based worker caps. Also verify the behavior with availableParallelism=1 edge cases.
PRA-7 Improvement correctness vitest.config.ts:33 Document this limitation in a comment. If future needs require dynamic scheduling, consider a function-based approach.
PRA-8 Improvement tests test/vitest-integration-scheduling.test.ts:1 Consider adding an integration test that verifies the Vitest project configuration behaves as expected, or document that the unit tests are sufficient because the logic is pure.
PRA-9 Improvement acceptance test/README.md:65 The README correctly documents that the acceptance clauses were met via a different design path (parallelization instead of subprocess removal). No code change needed, but ensure stakeholders agree this satisfies the original intent.
Review findings by urgency: 0 required fixes, 2 items to resolve/justify, 7 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: Forward list mock added to 'no restore intent' test case

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: None yet — add test verifying not-ready path doesn't depend on forward list
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: test/onboard-installer-restore-intent.test.ts lines 270-274 add forward list mock; comment at line 268 says 'host port occupancy is unrelated to the not-ready decision under test'

PRA-2 Resolve/justify — PR #6250 modifies the same test file — potential merge conflict or behavioral interference

  • Location: test/onboard-installer-restore-intent.test.ts:270
  • Category: scope
  • Problem: PR fix(onboard): diagnose incomplete custom plugin images #6250 ('fix(onboard): diagnose incomplete custom plugin images') also modifies test/onboard-installer-restore-intent.test.ts. Both PRs modify the same test fixture's mock setup. This PR adds a forward list mock at lines 270-274 in the 'no restore intent' test case. If both land without coordination, merge conflicts or behavioral interference in the restore-intent test coverage may occur.
  • Impact: Merge conflicts or test interference could break restore-intent coverage; both PRs modify the fixture's mock setup.
  • Recommended action: Coordinate with PR fix(onboard): diagnose incomplete custom plugin images #6250 author to ensure changes are compatible. Review PR fix(onboard): diagnose incomplete custom plugin images #6250's diff against this file to confirm no overlapping modifications to the same test cases.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check PR fix(onboard): diagnose incomplete custom plugin images #6250's diff for test/onboard-installer-restore-intent.test.ts and compare with the forward list mock addition in this PR (lines 270-274).
  • Missing regression test: No automated test can detect this; requires manual coordination before both PRs merge.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check PR fix(onboard): diagnose incomplete custom plugin images #6250's diff for test/onboard-installer-restore-intent.test.ts and compare with the forward list mock addition in this PR (lines 270-274).
  • Evidence: pr_review_drift_context shows openPrOverlaps[0] with sameFiles including this test file; this PR adds forward list mock at lines 270-274

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-3 Improvement — Exported resolveIntegrationProjectScheduling lacks JSDoc documentation

  • Location: test/helpers/integration-project-scheduling.ts:47
  • Category: docs
  • Problem: The new public helper function resolveIntegrationProjectScheduling lacks JSDoc documentation explaining its purpose, parameters, return value, and the scheduling rules it encodes (CI vs local, coverage serialization, worker cap bounds).
  • Impact: Future maintainers must read implementation to understand scheduling policy; no inline documentation for IDE hover or generated docs.
  • Suggested action: Add JSDoc to the exported function describing the scheduling policy: parallelizes only on local non-CI 'test' runs without coverage flags, caps workers at 4, honors --maxWorkers flag with numeric or percentage values.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read the exported function and verify it has comprehensive JSDoc.
  • Missing regression test: N/A - documentation improvement
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/helpers/integration-project-scheduling.ts exports resolveIntegrationProjectScheduling at line 47 with no JSDoc

PRA-4 Improvement — Fake lsof replaces host port binding — simplification already applied

  • Location: test/exit-code-user-error-surfaces.test.ts:149
  • Category: architecture
  • Problem: The dashboard-exhaustion fixture replaced 11 net.createServer calls and host-global port binding with a fake lsof script that reports ports as occupied from constants. Eliminates resource leaks, parallel worker interference, and cleanup complexity. No action needed — simplification already applied and verified.
  • Impact: More reliable test isolation; no host-global port binding that interferes with parallel integration workers.
  • Suggested action: No action needed — simplification already applied and verified.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run the onboard dashboard-port exhaustion test and verify it passes with the fake lsof and no actual port listeners.
  • Missing regression test: The existing test 'onboard dashboard-port exhaustion exits non-zero' already covers this.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/exit-code-user-error-surfaces.test.ts lines 149-177 replace net.createServer port binding with fake lsof script generating occupied ports from PORT_RANGE_START/END constants

PRA-5 Improvement — Forward list mock in 'no restore intent' test may be a workaround for incidental source behavior

  • Location: test/onboard-installer-restore-intent.test.ts:270
  • Category: correctness
  • Problem: The 'no restore intent' test case adds a forward list mock that returns a dashboard forward entry. This mock was not present in the original test and appears to be added to prevent the test from failing on a missing forward list call. However, the test comment says 'Keep dashboard allocation inside this restore-intent fixture; host port occupancy is unrelated to the not-ready decision under test.' This suggests the mock is a workaround for a side effect rather than testing the intended behavior.
  • Impact: If the forward list call is incidental to the not-ready decision, the test is masking a source code issue rather than verifying correct behavior. The mock may need to stay, but the source should be fixed to avoid the unnecessary call.
  • Suggested action: Verify that the forward list call is actually made in the code path under test, and that the mock return value is appropriate. If the forward list call is incidental to the not-ready decision, consider whether the test should instead verify the forward list is not called, or whether the source code should be refactored to avoid the unnecessary call.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run the test with the mock removed to see if it fails, and trace the code path in src/lib/onboard/not-ready-recreate.ts to understand why forward list is being called.
  • Missing regression test: Add a test that verifies the not-ready decision path does not depend on forward list state, or document why the forward list call is necessary.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/onboard-installer-restore-intent.test.ts lines 270-274 add forward list mock; comment at line 268 says 'host port occupancy is unrelated to the not-ready decision under test'

PRA-6 Improvement — Percentage-based --maxWorkers uses Math.round which may produce unexpected clamping

  • Location: test/helpers/integration-project-scheduling.ts:1
  • Category: correctness
  • Problem: The scheduling helper's parseWorkerCount function accepts percentage values (e.g., '10%') and uses Math.round((percentage / 100) * availableWorkers) then Math.max(1, ...). For small percentages this can produce unexpected results: 1% of 8 workers = 0.08 -> rounds to 0 -> clamped to 1.
  • Impact: Users specifying small percentages may get 1 worker instead of the intuitively expected 0 (which would be an error) or a different rounding behavior.
  • Suggested action: Add a comment clarifying the percentage rounding behavior, and consider whether Math.ceil would be more intuitive for percentage-based worker caps. Also verify the behavior with availableParallelism=1 edge cases.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check the test cases in test/vitest-integration-scheduling.test.ts for percentage edge cases (e.g., 1% of 8, 10% of 20).
  • Missing regression test: Add test cases for percentage edge cases: 1% of 1, 1% of 8, 99% of 100, etc.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/helpers/integration-project-scheduling.ts lines 13-22 implement parseWorkerCount with Math.round and Math.max(1, ...) clamping

PRA-7 Improvement — Integration project scheduling computed at config load time, fixed per run

  • Location: vitest.config.ts:33
  • Category: correctness
  • Problem: The integrationProjectScheduling constant is computed at config load time using process.env.npm_lifecycle_event and process.argv.slice(2). This means the scheduling decision is fixed at Vitest startup and cannot be changed per-project or per-run without restarting Vitest. This is acceptable for the current design but limits flexibility.
  • Impact: Cannot dynamically adjust scheduling for different project runs within the same Vitest invocation. Current design only needs the canonical local npm test vs CI/coverage distinction which is fixed at startup.
  • Suggested action: Document this limitation in a comment. If future needs require dynamic scheduling, consider a function-based approach.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Verify that running 'npm test' vs 'npx vitest run --project integration' produces the expected serial vs parallel behavior.
  • Missing regression test: The existing test vitest-integration-scheduling.test.ts covers the logic but not the Vitest config integration.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: vitest.config.ts lines 33-36 compute integrationProjectScheduling at module load time from process.env and process.argv

PRA-8 Improvement — Unit tests cover scheduling logic but not Vitest config integration

  • Location: test/vitest-integration-scheduling.test.ts:1
  • Category: tests
  • Problem: The new test file covers the scheduling logic well but does not test the interaction with the actual Vitest config (i.e., that the integration project actually runs in parallel when expected). The tests are pure unit tests of the helper function.
  • Impact: Logic is tested but integration with Vitest's project configuration is not verified. A config bug (e.g., wrong spread operator, missing project) would not be caught.
  • Suggested action: Consider adding an integration test that verifies the Vitest project configuration behaves as expected, or document that the unit tests are sufficient because the logic is pure.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run 'npm test' locally and observe that integration tests run with 4 workers (or the capped value).
  • Missing regression test: An integration test that spawns Vitest with the integration project and verifies worker count would be ideal but may be heavyweight.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/vitest-integration-scheduling.test.ts has 10 test cases covering logic but none verify actual Vitest project config behavior

PRA-9 Improvement — Acceptance clauses met via different design path (parallelization vs subprocess removal)

  • Location: test/README.md:65
  • Category: acceptance
  • Problem: The test/README.md documents the performance outcomes from perf(test): reduce test I/O to restore 2–5 minute full-suite runs #6245 and classifies the six initial hotspots. The acceptance criteria from perf(test): reduce test I/O to restore 2–5 minute full-suite runs #6245 included: (1) classify highest-cost files into unit-shaped vs process-contract, (2) convert onboard-selection.test.ts / onboard.test.ts to direct tests, (3) coordinate source-loader work with perf(ci): eliminate source-loader contention and hook timeouts in CLI coverage shards #6237 after subprocess removal. This PR takes a parallelization approach instead of subprocess conversion. The README acknowledges this: 'Different design path; outcome met but clause not directly addressed.'
  • Impact: Original acceptance criteria expected subprocess conversion; PR achieves performance target via parallelization instead. Stakeholders should confirm this satisfies the original intent.
  • Suggested action: The README correctly documents that the acceptance clauses were met via a different design path (parallelization instead of subprocess removal). No code change needed, but ensure stakeholders agree this satisfies the original intent.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Review the README's 'Highest-cost file disposition' table and verify each hotspot has a documented final disposition referencing specific PRs.
  • Missing regression test: N/A - documentation of acceptance outcome
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/README.md lines 65-85 document classification and disposition; lines 87-93 note parallelization approach
Simplification opportunities: 1 possible cut, net -25 lines possible

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-4 delete (test/exit-code-user-error-surfaces.test.ts:149): 11 net.createServer calls, async Promise.all port binding, server array tracking, afterEach server.close() loop
    • Replacement: Fake lsof bash script that validates port argument against OCCUPIED_PORT_CASES generated from constants
    • Net: -25 lines
    • Safety boundary: Real CLI binary, real onboard preflight, real diagnostic message, and non-zero exit contract all preserved
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — resolveWorkerCap percentage edge cases: 1% of 1, 1% of 8, 99% of 100. Scheduling logic is pure and well-unit-tested (10 cases). Fake lsof fixture preserves real CLI/preflight contract but runtime validation of parallel suite behavior (actual worker count, no interference) is recommended. Forward list mock workaround needs source verification.
  • PRA-T2 Runtime validation — not-ready-recreate.ts forward list call necessity verification. Scheduling logic is pure and well-unit-tested (10 cases). Fake lsof fixture preserves real CLI/preflight contract but runtime validation of parallel suite behavior (actual worker count, no interference) is recommended. Forward list mock workaround needs source verification.
  • PRA-T3 Runtime validation — Vitest integration project parallelism in practice (npm test runs with 4 workers). Scheduling logic is pure and well-unit-tested (10 cases). Fake lsof fixture preserves real CLI/preflight contract but runtime validation of parallel suite behavior (actual worker count, no interference) is recommended. Forward list mock workaround needs source verification.
  • PRA-T4 Unit tests cover scheduling logic but not Vitest config integration — Consider adding an integration test that verifies the Vitest project configuration behaves as expected, or document that the unit tests are sufficient because the logic is pure.
  • PRA-T5 Forward list mock added to 'no restore intent' test case — None yet — add test verifying not-ready path doesn't depend on forward list. test/onboard-installer-restore-intent.test.ts lines 270-274 add forward list mock; comment at line 268 says 'host port occupancy is unrelated to the not-ready decision under test'
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: Forward list mock added to 'no restore intent' test case

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: None yet — add test verifying not-ready path doesn't depend on forward list
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: test/onboard-installer-restore-intent.test.ts lines 270-274 add forward list mock; comment at line 268 says 'host port occupancy is unrelated to the not-ready decision under test'

PRA-2 Resolve/justify — PR #6250 modifies the same test file — potential merge conflict or behavioral interference

  • Location: test/onboard-installer-restore-intent.test.ts:270
  • Category: scope
  • Problem: PR fix(onboard): diagnose incomplete custom plugin images #6250 ('fix(onboard): diagnose incomplete custom plugin images') also modifies test/onboard-installer-restore-intent.test.ts. Both PRs modify the same test fixture's mock setup. This PR adds a forward list mock at lines 270-274 in the 'no restore intent' test case. If both land without coordination, merge conflicts or behavioral interference in the restore-intent test coverage may occur.
  • Impact: Merge conflicts or test interference could break restore-intent coverage; both PRs modify the fixture's mock setup.
  • Recommended action: Coordinate with PR fix(onboard): diagnose incomplete custom plugin images #6250 author to ensure changes are compatible. Review PR fix(onboard): diagnose incomplete custom plugin images #6250's diff against this file to confirm no overlapping modifications to the same test cases.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check PR fix(onboard): diagnose incomplete custom plugin images #6250's diff for test/onboard-installer-restore-intent.test.ts and compare with the forward list mock addition in this PR (lines 270-274).
  • Missing regression test: No automated test can detect this; requires manual coordination before both PRs merge.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check PR fix(onboard): diagnose incomplete custom plugin images #6250's diff for test/onboard-installer-restore-intent.test.ts and compare with the forward list mock addition in this PR (lines 270-274).
  • Evidence: pr_review_drift_context shows openPrOverlaps[0] with sameFiles including this test file; this PR adds forward list mock at lines 270-274

PRA-3 Improvement — Exported resolveIntegrationProjectScheduling lacks JSDoc documentation

  • Location: test/helpers/integration-project-scheduling.ts:47
  • Category: docs
  • Problem: The new public helper function resolveIntegrationProjectScheduling lacks JSDoc documentation explaining its purpose, parameters, return value, and the scheduling rules it encodes (CI vs local, coverage serialization, worker cap bounds).
  • Impact: Future maintainers must read implementation to understand scheduling policy; no inline documentation for IDE hover or generated docs.
  • Suggested action: Add JSDoc to the exported function describing the scheduling policy: parallelizes only on local non-CI 'test' runs without coverage flags, caps workers at 4, honors --maxWorkers flag with numeric or percentage values.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read the exported function and verify it has comprehensive JSDoc.
  • Missing regression test: N/A - documentation improvement
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/helpers/integration-project-scheduling.ts exports resolveIntegrationProjectScheduling at line 47 with no JSDoc

PRA-4 Improvement — Fake lsof replaces host port binding — simplification already applied

  • Location: test/exit-code-user-error-surfaces.test.ts:149
  • Category: architecture
  • Problem: The dashboard-exhaustion fixture replaced 11 net.createServer calls and host-global port binding with a fake lsof script that reports ports as occupied from constants. Eliminates resource leaks, parallel worker interference, and cleanup complexity. No action needed — simplification already applied and verified.
  • Impact: More reliable test isolation; no host-global port binding that interferes with parallel integration workers.
  • Suggested action: No action needed — simplification already applied and verified.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run the onboard dashboard-port exhaustion test and verify it passes with the fake lsof and no actual port listeners.
  • Missing regression test: The existing test 'onboard dashboard-port exhaustion exits non-zero' already covers this.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/exit-code-user-error-surfaces.test.ts lines 149-177 replace net.createServer port binding with fake lsof script generating occupied ports from PORT_RANGE_START/END constants

PRA-5 Improvement — Forward list mock in 'no restore intent' test may be a workaround for incidental source behavior

  • Location: test/onboard-installer-restore-intent.test.ts:270
  • Category: correctness
  • Problem: The 'no restore intent' test case adds a forward list mock that returns a dashboard forward entry. This mock was not present in the original test and appears to be added to prevent the test from failing on a missing forward list call. However, the test comment says 'Keep dashboard allocation inside this restore-intent fixture; host port occupancy is unrelated to the not-ready decision under test.' This suggests the mock is a workaround for a side effect rather than testing the intended behavior.
  • Impact: If the forward list call is incidental to the not-ready decision, the test is masking a source code issue rather than verifying correct behavior. The mock may need to stay, but the source should be fixed to avoid the unnecessary call.
  • Suggested action: Verify that the forward list call is actually made in the code path under test, and that the mock return value is appropriate. If the forward list call is incidental to the not-ready decision, consider whether the test should instead verify the forward list is not called, or whether the source code should be refactored to avoid the unnecessary call.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run the test with the mock removed to see if it fails, and trace the code path in src/lib/onboard/not-ready-recreate.ts to understand why forward list is being called.
  • Missing regression test: Add a test that verifies the not-ready decision path does not depend on forward list state, or document why the forward list call is necessary.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/onboard-installer-restore-intent.test.ts lines 270-274 add forward list mock; comment at line 268 says 'host port occupancy is unrelated to the not-ready decision under test'

PRA-6 Improvement — Percentage-based --maxWorkers uses Math.round which may produce unexpected clamping

  • Location: test/helpers/integration-project-scheduling.ts:1
  • Category: correctness
  • Problem: The scheduling helper's parseWorkerCount function accepts percentage values (e.g., '10%') and uses Math.round((percentage / 100) * availableWorkers) then Math.max(1, ...). For small percentages this can produce unexpected results: 1% of 8 workers = 0.08 -> rounds to 0 -> clamped to 1.
  • Impact: Users specifying small percentages may get 1 worker instead of the intuitively expected 0 (which would be an error) or a different rounding behavior.
  • Suggested action: Add a comment clarifying the percentage rounding behavior, and consider whether Math.ceil would be more intuitive for percentage-based worker caps. Also verify the behavior with availableParallelism=1 edge cases.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check the test cases in test/vitest-integration-scheduling.test.ts for percentage edge cases (e.g., 1% of 8, 10% of 20).
  • Missing regression test: Add test cases for percentage edge cases: 1% of 1, 1% of 8, 99% of 100, etc.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/helpers/integration-project-scheduling.ts lines 13-22 implement parseWorkerCount with Math.round and Math.max(1, ...) clamping

PRA-7 Improvement — Integration project scheduling computed at config load time, fixed per run

  • Location: vitest.config.ts:33
  • Category: correctness
  • Problem: The integrationProjectScheduling constant is computed at config load time using process.env.npm_lifecycle_event and process.argv.slice(2). This means the scheduling decision is fixed at Vitest startup and cannot be changed per-project or per-run without restarting Vitest. This is acceptable for the current design but limits flexibility.
  • Impact: Cannot dynamically adjust scheduling for different project runs within the same Vitest invocation. Current design only needs the canonical local npm test vs CI/coverage distinction which is fixed at startup.
  • Suggested action: Document this limitation in a comment. If future needs require dynamic scheduling, consider a function-based approach.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Verify that running 'npm test' vs 'npx vitest run --project integration' produces the expected serial vs parallel behavior.
  • Missing regression test: The existing test vitest-integration-scheduling.test.ts covers the logic but not the Vitest config integration.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: vitest.config.ts lines 33-36 compute integrationProjectScheduling at module load time from process.env and process.argv

PRA-8 Improvement — Unit tests cover scheduling logic but not Vitest config integration

  • Location: test/vitest-integration-scheduling.test.ts:1
  • Category: tests
  • Problem: The new test file covers the scheduling logic well but does not test the interaction with the actual Vitest config (i.e., that the integration project actually runs in parallel when expected). The tests are pure unit tests of the helper function.
  • Impact: Logic is tested but integration with Vitest's project configuration is not verified. A config bug (e.g., wrong spread operator, missing project) would not be caught.
  • Suggested action: Consider adding an integration test that verifies the Vitest project configuration behaves as expected, or document that the unit tests are sufficient because the logic is pure.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run 'npm test' locally and observe that integration tests run with 4 workers (or the capped value).
  • Missing regression test: An integration test that spawns Vitest with the integration project and verifies worker count would be ideal but may be heavyweight.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/vitest-integration-scheduling.test.ts has 10 test cases covering logic but none verify actual Vitest project config behavior

PRA-9 Improvement — Acceptance clauses met via different design path (parallelization vs subprocess removal)

  • Location: test/README.md:65
  • Category: acceptance
  • Problem: The test/README.md documents the performance outcomes from perf(test): reduce test I/O to restore 2–5 minute full-suite runs #6245 and classifies the six initial hotspots. The acceptance criteria from perf(test): reduce test I/O to restore 2–5 minute full-suite runs #6245 included: (1) classify highest-cost files into unit-shaped vs process-contract, (2) convert onboard-selection.test.ts / onboard.test.ts to direct tests, (3) coordinate source-loader work with perf(ci): eliminate source-loader contention and hook timeouts in CLI coverage shards #6237 after subprocess removal. This PR takes a parallelization approach instead of subprocess conversion. The README acknowledges this: 'Different design path; outcome met but clause not directly addressed.'
  • Impact: Original acceptance criteria expected subprocess conversion; PR achieves performance target via parallelization instead. Stakeholders should confirm this satisfies the original intent.
  • Suggested action: The README correctly documents that the acceptance clauses were met via a different design path (parallelization instead of subprocess removal). No code change needed, but ensure stakeholders agree this satisfies the original intent.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Review the README's 'Highest-cost file disposition' table and verify each hotspot has a documented final disposition referencing specific PRs.
  • Missing regression test: N/A - documentation of acceptance outcome
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: test/README.md lines 65-85 document classification and disposition; lines 87-93 note parallelization approach

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Add or justify PRA-T1 and any related test follow-ups.
Open items: 0 required · 0 warnings · 0 suggestions · 4 test follow-ups
Since last review: 1 prior item resolved · 0 still apply · 0 new items found

Action checklist

  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Evaluate vitest.config.ts with npm_lifecycle_event=test, no CI, and no coverage argv; assert the integration project has fileParallelism:true, maxWorkers:4, and sequence.groupOrder:1 while all expected non-live projects remain present.. The pure scheduling helper has useful positive and negative unit coverage, and the changed process fixtures retain behavioral assertions. The remaining confidence gap is that vitest.config.ts is a runtime configuration surface and the tests do not directly evaluate its process.env/process.argv wiring.
  • PRA-T2 Runtime validation — Evaluate vitest.config.ts with CI=true and npm_lifecycle_event=test; assert the integration project remains serialized with fileParallelism:false.. The pure scheduling helper has useful positive and negative unit coverage, and the changed process fixtures retain behavioral assertions. The remaining confidence gap is that vitest.config.ts is a runtime configuration surface and the tests do not directly evaluate its process.env/process.argv wiring.
  • PRA-T3 Runtime validation — Evaluate vitest.config.ts with local npm_lifecycle_event=test and coverage argv variants --coverage, --coverage=true, --coverage.enabled=true, and --coverage.provider=v8; assert the integration project remains serialized.. The pure scheduling helper has useful positive and negative unit coverage, and the changed process fixtures retain behavioral assertions. The remaining confidence gap is that vitest.config.ts is a runtime configuration surface and the tests do not directly evaluate its process.env/process.argv wiring.
  • PRA-T4 Runtime validation — Evaluate vitest.config.ts with --maxWorkers=1, --maxWorkers 2, --maxWorkers=10%, and --maxWorkers=8; assert the integration project receives the same bounded worker counts as resolveIntegrationProjectScheduling.. The pure scheduling helper has useful positive and negative unit coverage, and the changed process fixtures retain behavioral assertions. The remaining confidence gap is that vitest.config.ts is a runtime configuration surface and the tests do not directly evaluate its process.env/process.argv wiring.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@cv

cv commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

Advisor follow-up

All in-scope Nemotron suggestions are addressed in 45800b331:

  • malformed --maxWorkers values now fail clearly; numeric and percentage forms are supported and capped at four;
  • any --coverage* option fails closed to serialized integration execution;
  • the fake lsof cases are generated from PORT_RANGE_START/PORT_RANGE_END;
  • focused tests cover these cases, and process-level config checks confirmed invalid caps fail while --maxWorkers=1 reaches the integration project.

The #6250 overlap warning was also checked. #6250 changes the shared writeOkOpenshell helper import and the first restore-intent scenario's restore method. This PR adds a forward list response only to the second, no-restore-intent fixture. The hunks and behaviors are independent and compatible.

Why this PR closes #6245

#6245 is cumulative. This is the final scheduling and measurement change; the named conversions and boundary work were already merged:

Acceptance criterion Merged evidence
Classify highest-cost tests #6276 classified onboarding tests, #6280 classified route-swap/reconciliation tests, and #6417 handled the final ranked batch. nemoclaw-start.test.ts remains process-shaped because shell/startup behavior is its contract.
Convert onboard-selection and onboard #6276 changed both exact files and replaced unit-shaped children with typed seams. #6336 and #6383 further reduced provider-selection/onboarding process isolation while retaining boundary-sensitive witnesses.
Retain representative process contracts #6276 retained fail-closed, Bash, credential, and argv witnesses; #6280 retained CLI, timeout, listener, security, and cross-command contracts; #6285 retained real argv, exit, environment, stdin, secret, and executable-discovery launches; #6417 retained Docker/Kubectl, PowerShell-exit, and executable contracts.
Preserve non-live selection and coverage #6276 passed full npm test and all CLI ratchets; #6299 passed five-shard coverage with 11,496 tests; #6336 again passed every shard and ratchet. This PR preserves serialized CI/coverage routes and disjoint project membership.
Restore 2–5 minute feedback Issue baseline: 14:19.65 full suite and an 11:43 failed test-cli pre-commit path. #6270 removed full coverage from routine pre-commit while retaining manual and authoritative CI gates. Final cold Node 22 npm test: 3:52.14, 1,251 files, 13,875 passing, 39 skipped, zero failures—73% lower wall time despite a larger suite.
Avoid blanket timeout/retry/stale-dist workarounds The merged chain removed processes/loaders through typed/native seams. #6299 explicitly retired loader/cache churn; this PR adds bounded local workers and keeps contention-sensitive CI/coverage serialized.
Coordinate with #6237 after process removal Process-removal PRs #6276, #6279, #6280, #6282, and #6285 landed first. Then #6286 addressed #6237's 632-module hotspot; #6388 and #6415 ratcheted CLI createRequire files 32→28→27. This PR comes last.

Therefore, the absence of onboard-selection.test.ts and onboard.test.ts from this final diff is expected: those acceptance items were completed earlier. This PR supplies the previously missing supported-Node-22 full-suite measurement and reaches the issue's 2–5 minute target.

@cv

cv commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

Final-head advisor follow-up

The two Nemotron warnings have been investigated at 3a0ab3435:

  • PRA-1 / PRA-5: the forward list response is a faithful fixture boundary, not an invalid-state workaround. createSandboxWithBaseImageResolution intentionally resolves the registered sandbox's dashboard port before it evaluates the non-interactive not-ready outcome. A live registered my-assistant sandbox can therefore own the 18789 forward, as the first restore-intent scenario already models. Returning that ownership keeps unrelated real host listeners from changing this test's port allocation before it reaches the restore-intent assertion. If the production flow is ever reordered to reject missing restore intent before dashboard allocation, this fixture response can be removed; no source behavior needs to change for this PR.
  • PRA-2: fix(onboard): diagnose incomplete custom plugin images #6250 remains compatible at its latest head. It extracts the shared writeOkOpenshell helper and changes restoreSandboxState to restoreRecreatedSandboxState in the first scenario. This PR adds the forward-list response only in the second scenario. The hunks and behaviors do not overlap.

The remaining suggestions do not require another code revision:

  • PRA-6: Math.round deliberately matches Vitest 4.1.9's own percentage-worker resolver, including its minimum-one-worker clamp.
  • PRA-7: scheduling is intentionally resolved once when Vitest loads its configuration; environment and argv are fixed inputs for one test run.
  • PRA-3 / PRA-8 and PRA-T1PRA-T4: the pure 14-case matrix covers local, CI, coverage, focused, direct, numeric, percentage, malformed, and capped inputs. The actual config path was also exercised by the 3:52.03 full npm test, a process-level --maxWorkers=1 probe, and the final-head serialized CI coverage shards. A second config-loader harness would duplicate those signals without changing the policy.
  • PRA-9: perf(test): reduce test I/O to restore 2–5 minute full-suite runs #6245 was not satisfied by parallelization alone. The committed inventory records the subprocess-removal chain first, the sequenced perf(ci): eliminate source-loader contention and hook timeouts in CLI coverage shards #6237 loader work second, and bounded local scheduling as the final step.

GPT-5.5 now reports merge_as_is, CodeRabbit is clean, and the committed acceptance snapshot resolves the prior required finding.

@cv
cv merged commit 8ba3d0b into main Jul 7, 2026
50 checks passed
@cv
cv deleted the codex/perf-local-integration-parallelism branch July 7, 2026 23:06
@github-actions github-actions Bot added the v0.0.76 Release target label Jul 7, 2026
cv added a commit that referenced this pull request Jul 7, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Remove the dated #6245 performance snapshot now that the issue is
closed. This keeps repository documentation focused on current behavior
while retaining historical measurements in Git history, the issue, and
PR #6420.

## Related Issue
Follow-up cleanup after #6245.

## Changes
- Delete `test/README.md`, which contained host-specific timings,
historical hotspot classifications, and the completed optimization
narrative.
- Preserve active `#6245` regression-test suffixes and current Vitest
scheduling comments because they still document behavior rather than
historical run data.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: This deletes an unreferenced
archival Markdown snapshot and does not change code, test selection, or
runtime behavior.
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: No user-facing `docs/`
content changes; this removes an internal historical test artifact.
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect that behavior. Do
not rerun hook-covered checks. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification:
Repository scans found no remaining #6245 Markdown/MDX or recorded
timing strings; `npm run test:projects:check` reports 1,328 files
disjoint across 8 projects.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) —
command passed; Fern reported two advisory warnings, so this remains
intentionally unchecked.
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
* Removed the test suite README and its related performance notes,
measurement details, results tables, and coverage breakdowns.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Run the integration project as a bounded four-worker phase during the
canonical local `npm test`, while keeping CI, coverage, focused
integration, and direct Vitest runs serialized. Isolate two onboarding
fixtures from host-global dashboard ports so the parallel suite remains
deterministic. This is the final cumulative NVIDIA#6245 step after the named
onboarding conversions, representative process-contract work, and
sequenced loader cleanup already merged; the final clean-build Node 22
suite passes in 3:52.03.

## Related Issue
Closes NVIDIA#6245.

## Changes
- Replace the dashboard-exhaustion fixture's real host listeners with a
fake `lsof` while retaining the real CLI, preflight, diagnostic, and
non-zero exit contract.
- Give the restore-intent fixture an explicit existing dashboard forward
so unrelated host port occupancy cannot divert the behavior under test.
- Resolve integration scheduling from npm lifecycle, CI, coverage, and
worker-cap inputs: local `npm test` uses at most four workers in group
1, while every safety-sensitive route stays serial.
- Add a behavior matrix covering local, CI, coverage, focused, direct,
and explicit worker-throttle modes.
- Complete the cumulative NVIDIA#6245 acceptance path after NVIDIA#6276/NVIDIA#6336/NVIDIA#6383
converted the named onboarding hotspots, NVIDIA#6285/NVIDIA#6417 retained
representative process contracts, and NVIDIA#6286/NVIDIA#6299/NVIDIA#6388/NVIDIA#6415 sequenced
loader cleanup after process removal.
- Record the final host-specific timings, hotspot disposition, and
retained process-contract inventory in `test/README.md` as an advisory
acceptance snapshot rather than a permanent CI budget.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [x] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: Test fixtures and local
test-runner scheduling changed; NemoClaw commands, configuration,
runtime behavior, and CI/coverage workflows are unchanged.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: Independent final-diff
review confirmed that the fake `lsof` preserves the real
CLI/preflight/exit contract, the restore-intent assertions remain
intact, and resolved CI/coverage configurations remain serialized.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect the tested behavior.
Do not rerun hook-covered checks. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: Real
CLI exhaustion contract passed; restore-intent passed with all 11
dashboard ports deliberately occupied; scheduling matrix passed 14/14
through the lifecycle-triggered config; `npm run test:projects:check`
reported 1,327 files disjoint across 8 projects.
- [x] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result: Clean-build Node 22 `npm
test -- --reporter=blob` under the normal `umask 022` passed 1,251 files
and 13,879 tests with 39 skipped, 1 todo, and zero failures in 3:52.03,
down 73% from the issue's 14:19.65 baseline despite a larger suite. The
matching diff-scoped routine pre-commit stage passed in 13.95s. NVIDIA#6270
separately removed full coverage from routine pre-commit while
preserving manual and authoritative CI gates.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only)
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Integration test runs now use adaptive scheduling to speed up local
execution while keeping CI/focused runs serialized.

* **Bug Fixes**
* Improved reliability of onboarding regression coverage by simulating
dashboard port exhaustion in a hermetic way.
* Updated onboarding-related fixtures to better match the intended
readiness/exit behavior.

* **Tests**
* Added coverage for integration scheduling behavior (local caps,
invalid inputs, and CI/coverage scenarios).

* **Documentation**
* Added test-suite documentation with a local performance snapshot and
key test hotspots.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Remove the dated NVIDIA#6245 performance snapshot now that the issue is
closed. This keeps repository documentation focused on current behavior
while retaining historical measurements in Git history, the issue, and
PR NVIDIA#6420.

## Related Issue
Follow-up cleanup after NVIDIA#6245.

## Changes
- Delete `test/README.md`, which contained host-specific timings,
historical hotspot classifications, and the completed optimization
narrative.
- Preserve active `NVIDIA#6245` regression-test suffixes and current Vitest
scheduling comments because they still document behavior rather than
historical run data.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: This deletes an unreferenced
archival Markdown snapshot and does not change code, test selection, or
runtime behavior.
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: No user-facing `docs/`
content changes; this removes an internal historical test artifact.
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- Check each applicable item only when supported by the requested
evidence. Run targeted tests once per relevant change set and rerun
after later edits or hook autofixes that can affect that behavior. Do
not rerun hook-covered checks. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification:
Repository scans found no remaining NVIDIA#6245 Markdown/MDX or recorded
timing strings; `npm run test:projects:check` reports 1,328 files
disjoint across 8 projects.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) —
command passed; Fern reported two advisory warnings, so this remains
intentionally unchecked.
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
* Removed the test suite README and its related performance notes,
measurement details, results tables, and coverage breakdowns.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v0.0.76 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf(test): reduce test I/O to restore 2–5 minute full-suite runs

2 participants