Skip to content

Commit c2a7292

Browse files
authored
test(guardrails): consolidate growth checks in Vitest (NVIDIA#9354)
<!-- markdownlint-disable MD041 --> ## Summary Run the codebase growth policies as ordinary Vitest tests instead of separate scanners, evaluators, wrappers, and npm targets. The same tests now serve local changes and the trusted pull request workflow, reducing the implementation by 2,532 lines while preserving the five existing policies. ## Changes - Add one Vitest suite for new JavaScript files, `src/lib/onboard.ts` growth, test size budgets, test conditionals, and test loops. - Use one shared diff reader for local Git changes and pull request blobs. - Replace the seven-step growth workflow and the standalone pre-commit entry point with the same focused Vitest command. - Delete the superseded scanners, evaluators, wrapper tests, and two obsolete npm targets. Keep one bridge target for the base-trusted static action that executes against this PR, while the updated action uses the consolidated hook directly after merge. - Make the TypeScript requirement for new test files explicit in the contributor guide. ## Type of Change - [x] 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 - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] 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 - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run --project integration test/growth-guardrails.test.ts test/growth-guardrails-workflow-boundary.test.ts test/growth-guardrails-pr-blob-client.test.ts` (33 passed) - [ ] Applicable broad gate passed — `npm run check` completed with 29,085 passing tests and 40 unrelated host-sensitive failures caused by a foreign user systemd service, local platform facts, and a group-writable parent directory; the codebase growth hook and plugin coverage passed - [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) --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Consolidated code-growth checks into a unified validation suite. * Simplified validation for changed files and pull requests. * Updated pre-commit and changed-test workflows to use unified checks. * Removed redundant standalone scans and budget checks. * **Documentation** * Updated contribution guidance to require TypeScript for new test files. * **Tests** * Added coverage for growth limits, syntax patterns, renamed files, and pull-request changes. * Added direct validation of workflow configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
1 parent 24f471c commit c2a7292

27 files changed

Lines changed: 803 additions & 3335 deletions

.github/actions/ci-static-checks/action.yaml

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,17 +55,12 @@ runs:
5555
run: |
5656
npx prek run --all-files --stage pre-commit \
5757
--skip source-shape-test-budget \
58-
--skip test-file-size-budget \
5958
--skip test-skills-yaml
6059
6160
- name: Run source-shape budget
6261
shell: bash
6362
run: npm run source-shape:check
6463

65-
- name: Run test file size budget
66-
shell: bash
67-
run: npm run test-size:check
68-
6964
- name: Verify platform matrix is in sync
7065
shell: bash
7166
run: python3 scripts/generate-platform-docs.py --check

.github/workflows/codebase-growth-guardrails.yaml

Lines changed: 3 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -21,92 +21,6 @@ jobs:
2121
runs-on: ubuntu-latest
2222
timeout-minutes: 5
2323
steps:
24-
- name: Block newly added JavaScript files
25-
env:
26-
GH_TOKEN: ${{ github.token }}
27-
PR_NUMBER: ${{ github.event.pull_request.number }}
28-
REPO: ${{ github.repository }}
29-
run: |
30-
set -euo pipefail
31-
32-
rows="$(gh api --paginate "/repos/${REPO}/pulls/${PR_NUMBER}/files" \
33-
--jq '.[] | select((.filename | test("\\.(js|cjs|mjs)$")) and (.status == "added" or (.status == "renamed" and ((.previous_filename // "") | test("\\.(js|cjs|mjs)$") | not)))) | [.status, .filename, (.previous_filename // "")] | @tsv')"
34-
35-
if [ -z "$rows" ]; then
36-
echo "PASS: no newly added .js, .cjs, or .mjs files."
37-
exit 0
38-
fi
39-
40-
cat <<'EOF'
41-
FAIL: this PR adds JavaScript source files.
42-
43-
NemoClaw is standardizing on TypeScript for new Node.js code. Please
44-
use .ts for new source, test, and script files instead of .js, .cjs,
45-
or .mjs. Existing JavaScript files may still be modified or deleted.
46-
47-
Blocked files:
48-
EOF
49-
while IFS=$'\t' read -r file_status file_path previous_path; do
50-
if [ -n "$previous_path" ]; then
51-
echo " - ${file_path} (${file_status} from ${previous_path})"
52-
else
53-
echo " - ${file_path} (${file_status})"
54-
fi
55-
done <<< "$rows"
56-
exit 1
57-
58-
- name: Require src/lib/onboard.ts to be net-neutral or smaller
59-
env:
60-
GH_TOKEN: ${{ github.token }}
61-
PR_NUMBER: ${{ github.event.pull_request.number }}
62-
REPO: ${{ github.repository }}
63-
TARGET_FILE: src/lib/onboard.ts
64-
EXTRACTION_DIR: src/lib/onboard/
65-
run: |
66-
set -euo pipefail
67-
68-
# Intentionally hard-code TARGET_FILE in the jq expression. gh's jq
69-
# filter does not accept shell variables directly, and missing
70-
# previous_filename values must not match every file.
71-
rows="$(gh api --paginate "/repos/${REPO}/pulls/${PR_NUMBER}/files" \
72-
--jq '.[] | select(.filename == "src/lib/onboard.ts" or .previous_filename == "src/lib/onboard.ts") | [.additions, .deletions, .filename] | @tsv')"
73-
74-
if [ -z "$rows" ]; then
75-
echo "${TARGET_FILE} was not changed. New modules under ${EXTRACTION_DIR} are allowed."
76-
exit 0
77-
fi
78-
79-
additions=0
80-
deletions=0
81-
while IFS=$'\t' read -r file_additions file_deletions _file_path; do
82-
if [ -z "${file_additions:-}" ]; then
83-
continue
84-
fi
85-
additions=$((additions + file_additions))
86-
deletions=$((deletions + file_deletions))
87-
done <<< "$rows"
88-
89-
net=$((additions - deletions))
90-
echo "${TARGET_FILE}: +${additions}/-${deletions} (net ${net})"
91-
echo "Growth under ${EXTRACTION_DIR} is allowed; this budget applies only to ${TARGET_FILE}."
92-
93-
if [ "$additions" -le "$deletions" ]; then
94-
echo "PASS: ${TARGET_FILE} is net-neutral or smaller."
95-
exit 0
96-
fi
97-
98-
cat <<EOF
99-
FAIL: ${TARGET_FILE} grew by ${net} line(s).
100-
101-
${TARGET_FILE} is already about 12k lines. Please move new logic into
102-
focused modules under ${EXTRACTION_DIR}, or reduce ${TARGET_FILE} by at
103-
least as many lines as this PR adds there.
104-
105-
This check allows src/lib/onboard/** to grow. It only blocks net growth
106-
in the top-level onboard entrypoint.
107-
EOF
108-
exit 1
109-
11024
- name: Check out the trusted base revision
11125
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
11226
with:
@@ -116,32 +30,9 @@ jobs:
11630
- name: Install trusted dependencies
11731
run: npm ci --ignore-scripts --no-audit --no-fund
11832

119-
- name: Require changed test files to stay within size budget
120-
env:
121-
GH_TOKEN: ${{ github.token }}
122-
PR_NUMBER: ${{ github.event.pull_request.number }}
123-
REPO: ${{ github.repository }}
124-
BASE_SHA: ${{ github.event.pull_request.base.sha }}
125-
HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }}
126-
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
127-
run: |
128-
set -euo pipefail
129-
node --experimental-strip-types tools/growth-guardrails/test-size-budget.mts
130-
131-
- name: Require changed test files not to add if statements
132-
env:
133-
GH_TOKEN: ${{ github.token }}
134-
PR_NUMBER: ${{ github.event.pull_request.number }}
135-
REPO: ${{ github.repository }}
136-
BASE_SHA: ${{ github.event.pull_request.base.sha }}
137-
HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }}
138-
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
139-
run: |
140-
set -euo pipefail
141-
node --experimental-strip-types tools/growth-guardrails/test-conditionals.mts
142-
143-
- name: Require changed test files not to increase test-loop counts
33+
- name: Test codebase growth guardrails
14434
env:
35+
NEMOCLAW_GROWTH_PR: "1"
14536
GH_TOKEN: ${{ github.token }}
14637
PR_NUMBER: ${{ github.event.pull_request.number }}
14738
REPO: ${{ github.repository }}
@@ -150,4 +41,4 @@ jobs:
15041
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
15142
run: |
15243
set -euo pipefail
153-
node --experimental-strip-types tools/growth-guardrails/test-loops.mts
44+
npx vitest run --project integration test/growth-guardrails.test.ts

.pre-commit-config.yaml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -333,12 +333,12 @@ repos:
333333
files: ^(.+\.(test|spec)\.(js|ts|mjs|mts|cjs|cts)|scripts/find-source-shape-tests\.mts|ci/source-shape-test-budget\.json)$
334334
priority: 20
335335

336-
- id: test-file-size-budget
337-
name: Test file size budget
338-
entry: npm run test-size:check
336+
- id: codebase-growth-guardrails
337+
name: Codebase growth guardrails
338+
entry: npx vitest run --project integration test/growth-guardrails.test.ts
339339
language: system
340+
always_run: true
340341
pass_filenames: false
341-
files: ^(test/|src/.*\.(test|spec)\.(ts|js|mts|mjs|cts|cjs)$|nemoclaw/src/.*\.(test|spec)\.(ts|js|mts|mjs|cts|cjs)$|scripts/check-test-file-size-budget\.mts$|ci/test-file-size-budget\.json$)
342342
priority: 20
343343

344344
- id: test-skills-yaml

CONTRIBUTING.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ The repository is organized as follows.
490490

491491
All new source files must be TypeScript. Do not add new `.js` files to the project. When modifying an existing JavaScript file, prefer migrating it to TypeScript in the same PR.
492492

493-
Only a small CommonJS launcher/compatibility layer remains in `bin/`, while the main CLI implementation now lives in `src/lib/` and compiles to `dist/`. Tests in `test/` may remain ESM JavaScript for now but new test files should use TypeScript where practical.
493+
Only a small CommonJS launcher/compatibility layer remains in `bin/`, while the main CLI implementation now lives in `src/lib/` and compiles to `dist/`. Existing tests in `test/` may remain ESM JavaScript, but new test files must use TypeScript.
494494

495495
Shell scripts (`scripts/*.sh`) must pass ShellCheck and use `shfmt` formatting.
496496

ci/source-architecture-budget.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,6 @@
6262
"src/lib/actions/sandbox": 183,
6363
"src/lib/state": 39,
6464
"src/lib/inference": 63,
65-
"scripts": 46
65+
"scripts": 44
6666
}
6767
}

ci/source-shape-test-budget.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@
6363
},
6464
{
6565
"file": "test/growth-guardrails-workflow-boundary.test.ts",
66-
"test": "flags %s",
66+
"test": "runs the trusted Vitest guardrails against pull request data",
6767
"category": "security"
6868
},
6969
{

package.json

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
"test": "npm run clean:cli && npm --prefix nemoclaw run clean && npm run build:cli && npm --prefix nemoclaw run build && vitest run --project cli --project integration --project installer-integration --project package-contract --project plugin --project e2e-support",
2727
"test:spec": "npm test -- --reporter=tree",
2828
"test:fast": "npm run clean:cli && npm run catalog:compile && vitest run --project cli --project plugin --project e2e-support",
29-
"test:changed": "npm run catalog:compile && vitest run --changed --project cli --project plugin --project e2e-support",
29+
"test:changed": "npm run catalog:compile && vitest run --project integration test/growth-guardrails.test.ts && vitest run --changed --project cli --project plugin --project e2e-support",
3030
"test:watch": "npm run catalog:compile && vitest watch --project cli --project plugin --project e2e-support",
3131
"test:shuffle": "npm run catalog:compile && vitest run --project cli --project plugin --project e2e-support --sequence.shuffle.tests --coverage=false",
3232
"test:diagnose:leaks": "npm run catalog:compile && vitest run --project cli --project plugin --project e2e-support --detectAsyncLeaks --coverage=false --reporter=default --reporter=hanging-process",
@@ -66,9 +66,7 @@
6666
"type-safety:hotspots": "tsx scripts/type-safety-hotspots.mts",
6767
"source-shape:scan": "tsx scripts/find-source-shape-tests.mts --metrics",
6868
"source-shape:check": "tsx scripts/find-source-shape-tests.mts --check",
69-
"test-size:check": "tsx scripts/check-test-file-size-budget.mts",
70-
"test-conditionals:scan": "tsx scripts/find-test-conditionals.mts",
71-
"test-loops:scan": "tsx scripts/growth-guardrails/find-test-loops.mts",
69+
"test-size:check": "vitest run --project integration test/growth-guardrails.test.ts",
7270
"bump:version": "tsx scripts/bump-version.mts",
7371
"release:plan": "tsx scripts/release-plan.mts",
7472
"release:cut": "bash scripts/release-cut-tag.sh",

0 commit comments

Comments
 (0)