ci(test): clarify test loop remediation (#9263)

<!-- markdownlint-disable MD041 -->
## Summary

Clarifies the test-loop growth guardrail as a per-file count ratchet.
Failures now direct iteration for one behavior to a named helper outside
the test callback and independent rows to `it.each` or `test.each`,
without changing which loops the scanner detects.

## Changes

- Rename scanner, workflow, and evaluator output from table-test
candidates to test loops.
- Describe the ratchet as a per-file count check and provide both
supported remediation paths.
- Add coverage for named-helper iteration, report terminology, and the
exact entrypoint diagnostic.
- Update the trusted workflow step name and bind its exact source hash
in the workflow-boundary policy.

## 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:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: This internal contributor CI
guardrail does not change a user-facing NemoClaw command, configuration,
runtime workflow, or supported product behavior.
- [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: The trusted
`pull_request_target` workflow remains read-only, checks out the base
repository, parses pull-request blobs as data, and retains an exact
step-source hash verified by the workflow-boundary test.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Documentation Writer Review

- [x] Documentation writer subagent reviewed the completed changes
- Result: `no-docs-needed`
- Evidence: Reviewed all seven changed CI guardrail, scanner,
policy-evaluator, workflow-boundary, and test files. This internal
contributor guardrail refinement does not change a user-facing NemoClaw
command, configuration, runtime workflow, or supported product behavior.
- Agent: Codex Desktop
<!-- docs-review-head-sha: 1ca622e -->
<!-- docs-review-agents-blob-sha: e30afb2 -->

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit:
- Station profile/scenario:
- Result:
- Supporting evidence:

## 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/test-loops-scanner.test.ts
test/growth-guardrails-test-loops.test.ts
test/growth-guardrails-entrypoints.test.ts
test/growth-guardrails-workflow-boundary.test.ts` (4 files, 36 tests
passed)
- [ ] Applicable broad gate passed — `npm run check` completed the
structural, formatting, lint, repository, secret, YAML, shell,
container, E2E support, workflow, and plugin gates, then failed in
CLI/integration coverage on unrelated host-sensitive state (84 tests
across 10 files, including a foreign user systemd unit, writable Codex
parent directory, DGX Spark detection, and Git-fixture isolation).
- [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

* **Bug Fixes**
* Updated test-loop guardrails and diagnostics to consistently identify
increases in test loops.
* Improved guidance for resolving violations, including using linear
callbacks, named helpers, and parameterized tests.
* Confirmed loops in approved helper contexts and hooks are handled
correctly.

* **Tests**
* Expanded coverage for loop detection, reporting, exemptions, and
workflow enforcement.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Author: cv

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

@@ -140,7 +140,7 @@ jobs:
           set -euo pipefail
           node --experimental-strip-types tools/growth-guardrails/test-conditionals.mts
 
-      - name: Require changed test files not to add table-test candidate loops
+      - name: Require changed test files not to increase test-loop counts
         env:
           GH_TOKEN: ${{ github.token }}
           PR_NUMBER: ${{ github.event.pull_request.number }}

scripts/growth-guardrails/find-test-loops.mts

@@ -2,10 +2,10 @@
 // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 //
-// Finds `for` loops that make test cases or generated test definitions
-// iterative. Independent rows should use it.each or test.each so each failure
-// identifies one behavior. Required iteration can stay in a named helper
-// outside the test callback.
+// Finds `for` loops inside test callbacks and loops that generate test
+// definitions. Required iteration can stay in a named helper outside the test
+// callback. Independent rows should use it.each or test.each so each failure
+// identifies one behavior.
 
 import { existsSync, readdirSync, readFileSync, statSync } from "node:fs";
 import path from "node:path";
@@ -264,14 +264,14 @@ function formatContext(occurrence: TestLoopOccurrence): string {
 
 export function formatReport(report: TestLoopReport, options: Pick<CliOptions, "top">): string {
   const lines = [
-    `Scanned ${report.summary.scannedFiles} test files; found ${report.summary.loopCount} table-test candidate loop(s) in ${report.summary.filesWithLoops} file(s).`,
+    `Scanned ${report.summary.scannedFiles} test files; found ${report.summary.loopCount} test loop(s) in ${report.summary.filesWithLoops} file(s).`,
     "",
     "Top files by loop count:",
   ];
   for (const file of report.files.slice(0, options.top)) {
     lines.push(`- ${file.file}: loops=${file.count}`);
   }
-  lines.push("", "Table-test candidate loops:");
+  lines.push("", "Test loops:");
   for (const occurrence of report.occurrences.slice(0, options.top)) {
     lines.push(
       `- ${occurrence.file}:${occurrence.line}:${occurrence.column} [${formatContext(occurrence)}] ${occurrence.kind}`,

test/growth-guardrails-entrypoints.test.ts

@@ -94,7 +94,7 @@ describe("growth-guardrails executable entrypoints (#6953)", () => {
 
     expect(result.status).toBe(0);
     expect(result.stdout).toContain(
-      "PASS: changed test files did not add table-test candidate loops (0 at the latest PR commit vs 0 at base).",
+      "PASS: no changed test file increased its test-loop count (0 total at the latest PR commit vs 0 at base).",
     );
   });
 
@@ -106,9 +106,12 @@ describe("growth-guardrails executable entrypoints (#6953)", () => {
     ]);
 
     expect(result.status).toBe(1);
-    expect(result.stderr).toContain("FAIL: changed test files add table-test candidate loops.");
+    expect(result.stderr).toContain("FAIL: changed test files increase test-loop counts.");
     expect(result.stderr).toContain(
-      "test/a.test.ts: 1 table-test candidate loop(s), up from 0",
+      "test/a.test.ts: 1 test loop(s), up from 0",
+    );
+    expect(result.stderr).toContain(
+      "Move iteration needed for one behavior into a named helper outside the test callback. Use it.each or test.each when loop rows are independent cases.",
     );
   });
 

test/growth-guardrails-test-loops.test.ts

@@ -61,17 +61,17 @@ describe("growth-guardrails test-loops: pure policy", () => {
     displayName: path,
   });
 
-  it("flags a test file that adds a table-test candidate loop", () => {
+  it("flags a test file that adds a test loop", () => {
     const result = evaluateLoopViolations(
       [same("test/a.test.ts")],
       blobs({ "test/a.test.ts": NO_LOOP }),
       blobs({ "test/a.test.ts": ONE_LOOP }),
     );
-    expect(result.details).toEqual(["test/a.test.ts: 1 table-test candidate loop(s), up from 0"]);
+    expect(result.details).toEqual(["test/a.test.ts: 1 test loop(s), up from 0"]);
     expect([result.baseTotal, result.headTotal]).toEqual([0, 1]);
   });
 
-  it("passes a test file that removes table-test candidate loops", () => {
+  it("passes a test file that removes test loops", () => {
     const result = evaluateLoopViolations(
       [same("test/a.test.ts")],
       blobs({ "test/a.test.ts": TWO_LOOPS }),
@@ -103,7 +103,7 @@ describe("growth-guardrails test-loops: pure policy", () => {
       blobs({ "test/adder.test.ts": ONE_LOOP, "test/remover.test.ts": NO_LOOP }),
     );
     expect(result.details).toEqual([
-      "test/adder.test.ts: 1 table-test candidate loop(s), up from 0",
+      "test/adder.test.ts: 1 test loop(s), up from 0",
     ]);
     expect([result.baseTotal, result.headTotal]).toEqual([2, 1]);
   });
@@ -128,14 +128,14 @@ describe("growth-guardrails test-loops: orchestration", () => {
     REPO: "NVIDIA/NemoClaw",
   } as const;
 
-  it("fails a PR whose changed test adds a table-test candidate loop", async () => {
+  it("fails a PR whose changed test adds a test loop", async () => {
     const client = fakeClient([{ filename: "test/a.test.ts", status: "modified" }], {
       "NVIDIA/NemoClaw base test/a.test.ts": NO_LOOP,
       "fork/repo head test/a.test.ts": ONE_LOOP,
     });
     const result = await runTestLoops(client, ENV);
     expect(result.ok).toBe(false);
-    expect(result.details).toEqual(["test/a.test.ts: 1 table-test candidate loop(s), up from 0"]);
+    expect(result.details).toEqual(["test/a.test.ts: 1 test loop(s), up from 0"]);
   });
 
   it("ignores changed source files that are not tests", async () => {

test/test-loops-scanner.test.ts

@@ -3,7 +3,10 @@
 
 import { describe, expect, it } from "vitest";
 
-import { scanTextForTestLoops } from "../scripts/growth-guardrails/find-test-loops.mts";
+import {
+  formatReport,
+  scanTextForTestLoops,
+} from "../scripts/growth-guardrails/find-test-loops.mts";
 
 describe("test loop scanner", () => {
   it("detects each for-loop form inside a test callback", () => {
@@ -51,22 +54,35 @@ describe("test loop scanner", () => {
     });
   });
 
-  it("ignores fixture text and loop forms that do not represent table-test candidates", () => {
+  it("allows required iteration in a named helper outside the test callback", () => {
     const occurrences = scanTextForTestLoops(
       "test/virtual-helper-loops.test.ts",
       `
-        const fixture = \`for (const row of rows) { expect(row).toBeDefined(); }\`;
         function collect(values) {
           for (const value of values) consume(value);
         }
+        it("collects values", () => {
+          collect(values);
+          expect(values).toBeDefined();
+        });
+      `,
+    );
+
+    expect(occurrences).toEqual([]);
+  });
+
+  it("ignores fixture text, hooks, and loop forms outside the rule", () => {
+    const occurrences = scanTextForTestLoops(
+      "test/virtual-ignored-loops.test.ts",
+      `
+        const fixture = \`for (const row of rows) { expect(row).toBeDefined(); }\`;
         afterEach(() => {
           for (const resource of resources) resource.close();
         });
         it("waits", () => {
           while (pending()) wait();
           do { wait(); } while (pending());
           values.forEach(consume);
-          collect(values);
           expect(fixture).toContain("for");
         });
       `,
@@ -96,4 +112,23 @@ describe("test loop scanner", () => {
       contextName: "iterates in interpolation",
     });
   });
+
+  it("reports findings as test loops", () => {
+    const occurrences = scanTextForTestLoops(
+      "test/virtual-report.test.ts",
+      'it("iterates", () => { for (const value of values) consume(value); });',
+    );
+    const report = formatReport(
+      {
+        summary: { scannedFiles: 1, filesWithLoops: 1, loopCount: 1 },
+        files: [{ file: "test/virtual-report.test.ts", count: 1 }],
+        occurrences,
+      },
+      { top: 1 },
+    );
+
+    expect(report).toContain("found 1 test loop(s) in 1 file(s)");
+    expect(report).toContain("\nTest loops:\n");
+    expect(report).not.toContain("table-test candidate");
+  });
 });

tools/growth-guardrails/test-loops.mts

@@ -1,9 +1,9 @@
 // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 //
-// Trusted policy evaluator: a changed test file may not add table-test
-// candidate loops. The workflow reads pull-request blobs as data and parses
-// them with the TypeScript AST. It never executes pull-request code.
+// Trusted policy evaluator: a changed test file must not increase its test-loop
+// count. The workflow reads pull-request blobs as data and parses them with the
+// TypeScript AST. It never executes pull-request code.
 
 import { scanTextForTestLoops } from "../../scripts/growth-guardrails/find-test-loops.mts";
 import {
@@ -54,7 +54,7 @@ export function evaluateLoopViolations(
     headTotal += headCount;
     if (headCount > baseCount) {
       details.push(
-        `${change.headPath ?? change.displayName}: ${headCount} table-test candidate loop(s), up from ${baseCount}`,
+        `${change.headPath ?? change.displayName}: ${headCount} test loop(s), up from ${baseCount}`,
       );
     }
   }
@@ -120,13 +120,13 @@ async function main(): Promise<void> {
   const client = createPrBlobClient({ token: env.GH_TOKEN });
   const result = await runTestLoops(client, env);
   if (!result.ok) {
-    console.error("FAIL: changed test files add table-test candidate loops.");
+    console.error("FAIL: changed test files increase test-loop counts.");
     console.error(
-      `Changed test files contain ${result.headTotal} table-test candidate loop(s) at the latest PR commit vs ${result.baseTotal} at base.`,
+      `Across all changed test files: ${result.headTotal} test loop(s) at the latest PR commit vs ${result.baseTotal} at base.`,
     );
     console.error("");
     console.error(
-      "Test cases should stay linear. Use it.each or test.each for independent cases. Move iteration that represents one behavior into a named helper outside the test callback.",
+      "Keep test callbacks linear. Move iteration needed for one behavior into a named helper outside the test callback. Use it.each or test.each when loop rows are independent cases.",
     );
     console.error("");
     console.error("Files with increased test loop counts:");
@@ -136,7 +136,7 @@ async function main(): Promise<void> {
     process.exit(1);
   }
   console.log(
-    `PASS: changed test files did not add table-test candidate loops (${result.headTotal} at the latest PR commit vs ${result.baseTotal} at base).`,
+    `PASS: no changed test file increased its test-loop count (${result.headTotal} total at the latest PR commit vs ${result.baseTotal} at base).`,
   );
 }
 

tools/growth-guardrails/workflow-boundary.mts

@@ -67,8 +67,8 @@ const APPROVED_STEP_SHAPES = [
     sha256: "991a7036d27aeb45fde2f3178936fba6ac87b90200b79c933015f9d63525d3cf",
   },
   {
-    name: "Require changed test files not to add table-test candidate loops",
-    sha256: "6432b36d3c5ed34c648d782e5fc2302c15e76aea88b0ed319dca080ef6ef9431",
+    name: "Require changed test files not to increase test-loop counts",
+    sha256: "b31484bda2065644a63d1fb5fdd36d4a98f307e9b6514eeb236952e689a64106",
   },
 ] as const;