diff --git a/.changeset/happy-cougars-taste.md b/.changeset/happy-cougars-taste.md new file mode 100644 index 0000000000..a845151cc8 --- /dev/null +++ b/.changeset/happy-cougars-taste.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/.yarnrc.yml b/.yarnrc.yml index 99022ec059..47ad1d8965 100644 --- a/.yarnrc.yml +++ b/.yarnrc.yml @@ -39,6 +39,7 @@ catalog: oxc-resolver: ^11.0.0 oxfmt: ^0.72.0 oxlint: ^1.87.0 + parse-diff: ^0.11.0 qrcode: ^1.5.0 type-fest: ^4.0.0 typescript: ^7.0.2 diff --git a/AGENTS.md b/AGENTS.md index d65e03350c..6bde275200 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -142,8 +142,8 @@ One change file per feature/fix. No need for multiple entries when addressing PR feedback. Releases are automated via CI — on merge to `main`, the changesets action creates a release PR or publishes to npm. -Ignored packages (no changesets needed): `@rnx-kit/ignore`, `@rnx-kit/template`, -test apps. +Don't add change files for private packages (`"private": true`), e.g. +`@rnx-kit/template` and test apps. ## Key Packages @@ -210,9 +210,10 @@ constraints, align-deps, and readme issues. ## Code Style - Run `yarn format` and `yarn lint` before committing (CI enforces both) -- Don't format or lint by hand or call the underlying tools directly — the - `rnx-kit-scripts format` and `lint` commands (`scripts/src/commands/`) decide - which tools and configs are used +- Don't format, lint or build by hand or call the underlying tools (e.g. + `oxfmt`, `oxlint`, `tsc`) directly — the `rnx-kit-scripts format`, `lint` and + `build` commands (`scripts/src/commands/`) decide which tools and configs are + used - The formatter also sorts imports and `package.json` fields - Prefer consuming existing helpers over redesigning shared internals. Ask before adding new shared abstractions; optimize for total complexity, not diff --git a/evals/package.json b/evals/package.json new file mode 100644 index 0000000000..fa5dcfcf45 --- /dev/null +++ b/evals/package.json @@ -0,0 +1,42 @@ +{ + "name": "@rnx-kit/evals", + "version": "0.0.0", + "private": true, + "description": "Agent evals for the rnx-kit repository", + "homepage": "https://github.com/microsoft/rnx-kit/tree/main/evals#readme", + "license": "MIT", + "author": { + "name": "Microsoft Open Source", + "email": "microsoftopensource@users.noreply.github.com" + }, + "repository": { + "type": "git", + "url": "https://github.com/microsoft/rnx-kit", + "directory": "evals" + }, + "type": "module", + "scripts": { + "build": "rnx-kit-scripts build", + "eval": "npx --yes promptfoo@0.124.1 eval", + "format": "rnx-kit-scripts format", + "lint": "rnx-kit-scripts lint", + "promptfoo": "npx --yes promptfoo@0.124.1", + "test": "rnx-kit-scripts test" + }, + "dependencies": { + "@rnx-kit/agent-evals": "workspace:*", + "@rnx-kit/eslint-plugin": "workspace:*", + "@rnx-kit/tools-filesystem": "workspace:*", + "oxc-parser": "catalog:", + "oxlint": "catalog:", + "parse-diff": "catalog:" + }, + "devDependencies": { + "@rnx-kit/scripts": "workspace:*", + "@rnx-kit/tsconfig": "workspace:*", + "@types/node": "^24.0.0" + }, + "engines": { + "node": ">=24.11" + } +} diff --git a/evals/promptfooconfig.yaml b/evals/promptfooconfig.yaml new file mode 100644 index 0000000000..23fd332c0e --- /dev/null +++ b/evals/promptfooconfig.yaml @@ -0,0 +1,35 @@ +# yaml-language-server: $schema=https://promptfoo.dev/config-schema.json +description: rnx-kit session audit +prompts: + - "{{copilotSession}}" +providers: + # Replays a Copilot session from its event log in `$COPILOT_HOME` (default: + # `~/.copilot`) and compares the session's branch with the commit the + # session started from: the working tree if the branch is checked out, or + # else the tip of the branch, local or on `config.remote` (default: `origin`), + # in a temporary worktree. Looks up modified issues and pull requests, and + # the author of the pull request for the session's branch, with `gh` in + # `config.repository` (default: the GitHub repository of the remote). + - id: file://src/provider.ts + label: copilot-session +defaultTest: + vars: + # ID of the session to audit, e.g. `--var copilotSession=`. If empty, + # audits the most recently updated session in this repository that is not + # in use. Not named `session` because promptfoo redacts fields with that + # name. + copilotSession: "" + options: + # Grades `llm-rubric` assertions with Copilot; requires a GitHub token + # with Copilot access in `COPILOT_GITHUB_TOKEN`, `GH_TOKEN` or + # `GITHUB_TOKEN`, or a `gh auth login` session. Pass `--grader` to use + # another provider instead. + provider: file://src/grader.ts +tests: file://tests/*.yaml +extensions: + # Prints each test with the reasons it failed, instead of promptfoo's table. + # Pass `--table` to show the table as well, or run `yarn view` to browse the + # results. + - file://src/report.ts:afterAll +commandLineOptions: + table: false diff --git a/evals/src/audit.ts b/evals/src/audit.ts new file mode 100644 index 0000000000..c77b6428a8 --- /dev/null +++ b/evals/src/audit.ts @@ -0,0 +1,98 @@ +import { + changedPackages, + changesets, + ranCommand, + result, + toAgentRun, + tryReadJSON, + workspacePackages, +} from "@rnx-kit/agent-evals"; +import type { AgentRun, GradingResult } from "@rnx-kit/agent-evals"; +import * as path from "node:path"; +import parseDiff from "parse-diff"; + +const DEPENDENCY_LINE = /^[+-]\s*"[^"]+":\s*"([\^~]?\d|[a-z]+:)/; + +/** + * Passes if the agent created new workspace packages with `yarn new-package`. + */ +export function newPackagesScaffolded( + output: AgentRun | string +): GradingResult { + const run = toAgentRun(output); + const added = Array.from(workspacePackages(run.workdir).values()).filter( + (dir) => + run.files.some( + (file) => file.status === "added" && file.path === `${dir}/package.json` + ) + ); + if (added.length === 0) { + return result([], "Not applicable: no packages were added"); + } + + const check = ranCommand(run, { + config: { pattern: "^yarn new-package\\b" }, + }); + return check.pass + ? check + : result([`Added ${added.join(", ")} without 'yarn new-package'`], ""); +} + +/** + * Passes if dependency changes in `docsite/package.json` are installed in + * `docsite/yarn.lock`, which is separate from the root lockfile. + */ +export function docsiteDependencies(output: AgentRun | string): GradingResult { + const run = toAgentRun(output); + const manifest = parseDiff(run.diff).find( + (file) => file.to === "docsite/package.json" + ); + const changed = manifest?.chunks.some((chunk) => + chunk.changes.some( + (change) => + change.type !== "normal" && DEPENDENCY_LINE.test(change.content) + ) + ); + if (!changed) { + return result( + [], + "Not applicable: no dependencies changed in docsite/package.json" + ); + } + return run.files.some((file) => file.path === "docsite/yarn.lock") + ? result([], "docsite/yarn.lock changed") + : result(["Changed dependencies without updating docsite/yarn.lock"], ""); +} + +/** + * Passes if the agent added change files releasing all published packages it + * changed, and none if it did not change any. No change file may release a + * private package. + * + * Note: Changes that do not affect the published package, e.g. to tests or + * docs, do not need a change file, but Changesets does not tell them apart. + */ +export function changesetsMatchChanges( + output: AgentRun | string +): GradingResult { + const run = toAgentRun(output); + const isChangeset = (file: { path: string }) => + file.path.startsWith(".changeset/"); + const published = changedPackages(run, (file) => !isChangeset(file)) + .filter(({ manifest }) => !manifest.private) + .map(({ name }) => name); + const privatePackages: string[] = []; + for (const [name, dir] of workspacePackages(run.workdir)) { + if (tryReadJSON(path.join(run.workdir, dir, "package.json"))?.private) { + privatePackages.push(name); + } + } + return changesets(run, { + config: { + count: published.length > 0 ? 1 : 0, + atLeast: published.length > 0, + packages: published, + forbidden: privatePackages, + }, + }); +} diff --git a/evals/src/configs/oxlint/types-only.json b/evals/src/configs/oxlint/types-only.json new file mode 100644 index 0000000000..5bf54e91c6 --- /dev/null +++ b/evals/src/configs/oxlint/types-only.json @@ -0,0 +1,9 @@ +{ + "$schema": "../../../node_modules/oxlint/configuration_schema.json", + "plugins": [], + "categories": { "correctness": "off" }, + "jsPlugins": [ + { "name": "@rnx-kit", "specifier": "@rnx-kit/eslint-plugin/rules" } + ], + "rules": { "@rnx-kit/type-definitions-only": "error" } +} diff --git a/evals/src/grader.ts b/evals/src/grader.ts new file mode 100644 index 0000000000..36749aeea8 --- /dev/null +++ b/evals/src/grader.ts @@ -0,0 +1,2 @@ +// oxlint-disable-next-line import/no-default-export +export { default } from "@rnx-kit/agent-evals/grader"; diff --git a/evals/src/index.ts b/evals/src/index.ts new file mode 100644 index 0000000000..749cb32ffe --- /dev/null +++ b/evals/src/index.ts @@ -0,0 +1,15 @@ +export { + didNotRunCommand, + forGrader, + matchesContent, + notModifiedOnGitHub, + ranChecksForChanges, +} from "@rnx-kit/agent-evals"; +export { + changesetsMatchChanges, + docsiteDependencies, + newPackagesScaffolded, +} from "./audit.ts"; +export { noInlineTypeSpecifiers } from "./lint.ts"; +export { passesOxlint } from "./oxlint.ts"; +export { usesPackageTestRunner } from "./tests.ts"; diff --git a/evals/src/lint.ts b/evals/src/lint.ts new file mode 100644 index 0000000000..b7b0797aa2 --- /dev/null +++ b/evals/src/lint.ts @@ -0,0 +1,85 @@ +import { + addedLines, + existingFiles, + requireConfig, + result, + toAgentRun, +} from "@rnx-kit/agent-evals"; +import type { + AgentRun, + AssertionContext, + GradingResult, +} from "@rnx-kit/agent-evals"; +import { parseSync } from "oxc-parser"; + +/** + * Configuration for `noInlineTypeSpecifiers`. + */ +export type TypeSpecifierMatcher = { + /** Regular expression matching the paths of the files to check */ + files: string; +}; + +/** + * Passes if the matching files do not use inline `type` specifiers in import + * or export statements, e.g. `import { type A, b }`, on lines added in the + * diff. Passes trivially if no files match. + */ +export function noInlineTypeSpecifiers( + output: AgentRun | string, + context: AssertionContext +): GradingResult { + const run = toAgentRun(output); + const pathPattern = new RegExp(requireConfig(context).files); + const added = addedLines(run.diff); + + const failures: string[] = []; + const checked: string[] = []; + for (const file of existingFiles(run)) { + if (!pathPattern.test(file.path)) { + continue; + } + + const lines = added.get(file.path); + if (!lines?.size) { + continue; + } + + checked.push(file.path); + const content = file.content ?? ""; + const { program, errors } = parseSync(file.path, content); + for (const { message } of errors) { + failures.push(`${file.path}: ${message}`); + } + + // Only top-level statements are checked; imports and exports nested in + // ambient module declarations (`declare module "…" {}`) are not. Lint + // allows these; `typescript/no-namespace` only rejects named namespaces. + for (const node of program.body) { + let keyword: string; + let specifiers: { start: number }[]; + if (node.type === "ImportDeclaration") { + keyword = "import"; + specifiers = node.specifiers.filter( + (s) => s.type === "ImportSpecifier" && s.importKind === "type" + ); + } else if (node.type === "ExportNamedDeclaration") { + keyword = "export"; + specifiers = node.specifiers.filter((s) => s.exportKind === "type"); + } else { + continue; + } + + for (const { start } of specifiers) { + const line = content.substring(0, start).split("\n").length; + if (lines.has(line)) { + failures.push( + `${file.path}:${line}: Use a separate \`${keyword} type\` statement` + ); + } + } + } + } + + return result(failures, `Checked ${checked.join(", ") || "(none)"}`); +} diff --git a/evals/src/oxlint.ts b/evals/src/oxlint.ts new file mode 100644 index 0000000000..2c2486d412 --- /dev/null +++ b/evals/src/oxlint.ts @@ -0,0 +1,97 @@ +import { + existingFiles, + requireConfig, + result, + toAgentRun, +} from "@rnx-kit/agent-evals"; +import type { + AgentRun, + AssertionContext, + GradingResult, +} from "@rnx-kit/agent-evals"; +import { spawnSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; + +/** + * Configuration for `passesOxlint`. + */ +export type OxlintMatcher = { + /** Regular expression matched against changed file paths */ + files: string; + /** Path to an oxlint config, relative to `src/configs` */ + config: string; +}; + +type OxlintReport = { + number_of_files: number; + diagnostics: { + message: string; + code: string; + filename: string; + labels: { span: { line: number } }[]; + }[]; +}; + +const OXLINT = fileURLToPath( + new URL("./cli.js", import.meta.resolve("oxlint")) +); +const OXLINT_TIMEOUT_MS = 60 * 1000; + +/** + * Passes if the matching changed files pass oxlint with the configured config. + * Passes trivially if no files match. Nested configs and ignore files in the + * checkout are ignored. + * + * Note: `spawnSync` blocks the event loop, which stalls other tests that + * promptfoo runs concurrently. Switch to an async spawn if this becomes a + * bottleneck; promptfoo accepts assertions that return a promise. + */ +export function passesOxlint( + output: AgentRun | string, + context: AssertionContext +): GradingResult { + const run = toAgentRun(output); + const { files, config } = requireConfig(context); + const pattern = new RegExp(files); + const matching = existingFiles(run) + .filter((file) => pattern.test(file.path)) + .map((file) => file.path); + if (matching.length === 0) { + return result([], "No matching files changed"); + } + + const configPath = fileURLToPath( + new URL(`configs/${config}`, import.meta.url) + ); + const { error, status, stdout } = spawnSync( + process.execPath, + [ + OXLINT, + "--config", + configPath, + "--no-ignore", + "--format", + "json", + ...matching, + ], + { cwd: run.workdir, encoding: "utf-8", timeout: OXLINT_TIMEOUT_MS } + ); + if (error || (status !== 0 && status !== 1)) { + throw new Error( + `oxlint failed: ${error?.message ?? `exit code ${status}`}` + ); + } + + const { diagnostics, number_of_files } = JSON.parse(stdout) as OxlintReport; + if (number_of_files !== matching.length) { + throw new Error( + `oxlint checked ${number_of_files} of ${matching.length} files` + ); + } + + const failures = diagnostics.map(({ code, filename, labels, message }) => { + const line = labels[0]?.span.line; + return `${filename}${line ? `:${line}` : ""}: ${code}: ${message}`; + }); + return result(failures, `${matching.join(", ")} passed oxlint`); +} diff --git a/evals/src/provider.ts b/evals/src/provider.ts new file mode 100644 index 0000000000..675c8f2596 --- /dev/null +++ b/evals/src/provider.ts @@ -0,0 +1,2 @@ +// oxlint-disable-next-line import/no-default-export +export { default } from "@rnx-kit/agent-evals/provider"; diff --git a/evals/src/report.ts b/evals/src/report.ts new file mode 100644 index 0000000000..c60d16f034 --- /dev/null +++ b/evals/src/report.ts @@ -0,0 +1 @@ +export { afterAll } from "@rnx-kit/agent-evals/report"; diff --git a/evals/src/tests.ts b/evals/src/tests.ts new file mode 100644 index 0000000000..ff98be82bf --- /dev/null +++ b/evals/src/tests.ts @@ -0,0 +1,73 @@ +import { + addedLines, + existingFiles, + owningPackageDir, + result, + selectLines, + toAgentRun, + workspacePackages, +} from "@rnx-kit/agent-evals"; +import type { AgentRun, GradingResult } from "@rnx-kit/agent-evals"; +import { readJSONFileSync } from "@rnx-kit/tools-filesystem"; +import * as fs from "node:fs"; +import * as path from "node:path"; + +const TEST_FILE = /\.test\.[cm]?[jt]sx?$/; +const JEST_API = /\bexpect\(|\bjest\.|@jest\/globals/; +const NODE_API = /from ["']node:(assert\/strict|test)/; + +/** + * Passes if every changed test file is written for the test runner of its + * package: Jest if the package has a `jest` field in `package.json` or a + * `jest.config.js`, otherwise the Node.js test runner. Passes trivially if no + * test files changed. + * + * Note: This mirrors `useJest()` in `scripts/src/commands/test.js`. + */ +export function usesPackageTestRunner( + output: AgentRun | string +): GradingResult { + const run = toAgentRun(output); + const tests = existingFiles(run).filter((file) => { + return ( + path.win32.basename(path.win32.dirname(file.path)) === "test" && + TEST_FILE.test(path.win32.basename(file.path)) + ); + }); + if (tests.length === 0) { + return result([], "Not applicable: no test files changed"); + } + + const added = addedLines(run.diff); + const dirs = Array.from(workspacePackages(run.workdir).values()); + const failures: string[] = []; + for (const file of tests) { + const dir = path.join(run.workdir, owningPackageDir(dirs, file.path)); + const jest = + fs.existsSync(path.join(dir, "jest.config.js")) || + Boolean(readJSONFileSync(path.join(dir, "package.json")).jest); + const content = file.content ?? ""; + const newLines = selectLines(file, added); + if (jest) { + if (NODE_API.test(newLines)) { + failures.push(`${file.path}: uses 'node:test' in a Jest package`); + } + } else { + if (!NODE_API.test(content)) { + failures.push( + `${file.path}: missing imports from 'node:test' and 'node:assert/strict'` + ); + } + if (JEST_API.test(newLines)) { + failures.push( + `${file.path}: uses Jest in a Node.js test runner package` + ); + } + } + } + + return result( + failures, + `Checked ${tests.map((file) => file.path).join(", ")}` + ); +} diff --git a/evals/test/__fixtures__/oxlint-no-plusplus.json b/evals/test/__fixtures__/oxlint-no-plusplus.json new file mode 100644 index 0000000000..b89a85da70 --- /dev/null +++ b/evals/test/__fixtures__/oxlint-no-plusplus.json @@ -0,0 +1 @@ +{ "rules": { "no-plusplus": "error" } } diff --git a/evals/test/__fixtures__/workspace/package.json b/evals/test/__fixtures__/workspace/package.json new file mode 100644 index 0000000000..987cff1410 --- /dev/null +++ b/evals/test/__fixtures__/workspace/package.json @@ -0,0 +1 @@ +{ "name": "fixture", "private": true, "workspaces": ["packages/*"] } diff --git a/evals/test/__fixtures__/workspace/packages/jest-config/jest.config.js b/evals/test/__fixtures__/workspace/packages/jest-config/jest.config.js new file mode 100644 index 0000000000..ff8b4c5632 --- /dev/null +++ b/evals/test/__fixtures__/workspace/packages/jest-config/jest.config.js @@ -0,0 +1 @@ +export default {}; diff --git a/evals/test/__fixtures__/workspace/packages/jest-config/package.json b/evals/test/__fixtures__/workspace/packages/jest-config/package.json new file mode 100644 index 0000000000..1a1f0ed760 --- /dev/null +++ b/evals/test/__fixtures__/workspace/packages/jest-config/package.json @@ -0,0 +1 @@ +{ "name": "jest-config", "version": "0.0.0" } diff --git a/evals/test/__fixtures__/workspace/packages/jest-field/package.json b/evals/test/__fixtures__/workspace/packages/jest-field/package.json new file mode 100644 index 0000000000..2f8f3c5a86 --- /dev/null +++ b/evals/test/__fixtures__/workspace/packages/jest-field/package.json @@ -0,0 +1 @@ +{ "name": "jest-field", "version": "0.0.0", "jest": {} } diff --git a/evals/test/__fixtures__/workspace/packages/node-pkg/package.json b/evals/test/__fixtures__/workspace/packages/node-pkg/package.json new file mode 100644 index 0000000000..df45748099 --- /dev/null +++ b/evals/test/__fixtures__/workspace/packages/node-pkg/package.json @@ -0,0 +1 @@ +{ "name": "node-pkg", "version": "0.0.0" } diff --git a/evals/test/__fixtures__/workspace/packages/node-pkg/src/index.ts b/evals/test/__fixtures__/workspace/packages/node-pkg/src/index.ts new file mode 100644 index 0000000000..3c6abf90f5 --- /dev/null +++ b/evals/test/__fixtures__/workspace/packages/node-pkg/src/index.ts @@ -0,0 +1,5 @@ +export function next(i: number): number { + let j = i; + j++; + return j; +} diff --git a/evals/test/__fixtures__/workspace/packages/node-pkg/src/types.ts b/evals/test/__fixtures__/workspace/packages/node-pkg/src/types.ts new file mode 100644 index 0000000000..d4e575f16a --- /dev/null +++ b/evals/test/__fixtures__/workspace/packages/node-pkg/src/types.ts @@ -0,0 +1 @@ +export type Count = number; diff --git a/evals/test/__fixtures__/workspace/packages/node-pkg/test/__fixtures__/nested/package.json b/evals/test/__fixtures__/workspace/packages/node-pkg/test/__fixtures__/nested/package.json new file mode 100644 index 0000000000..4c5741a562 --- /dev/null +++ b/evals/test/__fixtures__/workspace/packages/node-pkg/test/__fixtures__/nested/package.json @@ -0,0 +1 @@ +{ "name": "nested", "version": "0.0.0", "jest": {} } diff --git a/evals/test/audit.test.ts b/evals/test/audit.test.ts new file mode 100644 index 0000000000..9328d1b068 --- /dev/null +++ b/evals/test/audit.test.ts @@ -0,0 +1,41 @@ +import type { AgentChangedFile } from "@rnx-kit/agent-evals"; +import { equal } from "node:assert/strict"; +import { describe, it } from "node:test"; +import { changesetsMatchChanges } from "../src/audit.ts"; +import { added, makeRun } from "./helpers.ts"; + +function changeset(name: string, ...packages: string[]): AgentChangedFile { + const releases = packages.map((pkg) => `"${pkg}": patch`); + return added( + `.changeset/${name}.md`, + `---\n${releases.join("\n")}\n---\n\nFix\n` + ); +} + +describe("changesetsMatchChanges()", () => { + it("passes with change files for every changed published package", () => { + const files = [ + added("packages/node-pkg/src/index.ts", ""), + changeset("a", "node-pkg"), + changeset("b", "node-pkg"), + ]; + equal(changesetsMatchChanges(makeRun({ files })).pass, true); + }); + + it("fails without change files for changed published packages", () => { + const files = [added("packages/node-pkg/src/index.ts", "")]; + const result = changesetsMatchChanges(makeRun({ files })); + equal(result.pass, false); + equal( + result.reason, + "Expected at least 1 change file(s), found 0: \nNo change file releases node-pkg" + ); + }); + + it("fails with change files if no published packages changed", () => { + const files = [added("README.md", ""), changeset("a", "node-pkg")]; + const result = changesetsMatchChanges(makeRun({ files })); + equal(result.pass, false); + equal(result.reason, "Expected 0 change file(s), found 1: .changeset/a.md"); + }); +}); diff --git a/evals/test/helpers.ts b/evals/test/helpers.ts new file mode 100644 index 0000000000..f753dc5e4f --- /dev/null +++ b/evals/test/helpers.ts @@ -0,0 +1,44 @@ +import type { AgentChangedFile, AgentRun } from "@rnx-kit/agent-evals"; +import { fileURLToPath } from "node:url"; + +export const WORKSPACE = fileURLToPath( + new URL("__fixtures__/workspace", import.meta.url) +); + +export function makeRun(run: Partial = {}): AgentRun { + return { + commands: [], + toolCalls: [], + files: [], + diff: "", + finalMessage: "", + workdir: WORKSPACE, + ...run, + }; +} + +export function added(path: string, content: string): AgentChangedFile { + return { path, status: "added", content }; +} + +/** + * Returns a unified diff that adds the specified lines (1-based) of the file. + */ +export function diffAdding( + { path, content = "" }: AgentChangedFile, + lines?: number[] +): string { + const all = content.split("\n"); + const body = all.map((line, i) => + !lines || lines.includes(i + 1) ? `+${line}` : ` ${line}` + ); + const oldCount = body.filter((line) => line.startsWith(" ")).length; + return [ + `diff --git a/${path} b/${path}`, + `--- a/${path}`, + `+++ b/${path}`, + `@@ -1,${oldCount} +1,${all.length} @@`, + ...body, + "", + ].join("\n"); +} diff --git a/evals/test/lint.test.ts b/evals/test/lint.test.ts new file mode 100644 index 0000000000..e8c1c197b9 --- /dev/null +++ b/evals/test/lint.test.ts @@ -0,0 +1,87 @@ +import type { AgentChangedFile } from "@rnx-kit/agent-evals"; +import { equal } from "node:assert/strict"; +import { describe, it } from "node:test"; +import { noInlineTypeSpecifiers } from "../src/lint.ts"; +import { added, diffAdding, makeRun } from "./helpers.ts"; + +describe("noInlineTypeSpecifiers()", () => { + function check( + files: AgentChangedFile[], + config: Partial< + Parameters[1]["config"] + > = {}, + diff = files.map((file) => diffAdding(file)).join("") + ) { + return noInlineTypeSpecifiers(makeRun({ files, diff }), { + config: { files: "\\.ts$", ...config }, + }); + } + + it("passes for separate type imports and exports", () => { + const file = added( + "a.ts", + [ + 'import type { A } from "a";', + 'import { b } from "b";', + 'export type { C } from "c";', + "export { b };", + ].join("\n") + ); + const result = check([file]); + equal(result.pass, true); + equal(result.reason, "Checked a.ts"); + }); + + it("fails for inline type specifiers in imports and exports", () => { + const file = added( + "a.ts", + [ + 'import { type A, b } from "a";', + 'export { type C, d } from "c";', + "type E = string;", + "export { type E };", + ].join("\n") + ); + const result = check([file]); + equal(result.pass, false); + equal( + result.reason, + [ + "a.ts:1: Use a separate `import type` statement", + "a.ts:2: Use a separate `export type` statement", + "a.ts:4: Use a separate `export type` statement", + ].join("\n") + ); + }); + + it("only checks matching, existing files", () => { + const result = check([ + added("a.js", 'import { type A } from "a";'), + { path: "b.ts", status: "deleted" }, + ]); + equal(result.pass, true); + equal(result.reason, "Checked (none)"); + }); + + it("reports parse errors", () => { + const result = check([added("a.ts", "import {")]); + equal(result.pass, false); + equal(result.reason.startsWith("a.ts: "), true); + }); + + it("only checks added lines", () => { + const file = added( + "a.ts", + 'import { type A } from "a";\nimport { type B } from "b";' + ); + + const result = check([file], {}, diffAdding(file, [2])); + equal(result.pass, false); + equal(result.reason, "a.ts:2: Use a separate `import type` statement"); + + const other = added("b.ts", ""); + const skipped = check([file, other], {}, diffAdding(other)); + equal(skipped.pass, true); + equal(skipped.reason, "Checked b.ts"); + }); +}); diff --git a/evals/test/oxlint.test.ts b/evals/test/oxlint.test.ts new file mode 100644 index 0000000000..5708d0a7a6 --- /dev/null +++ b/evals/test/oxlint.test.ts @@ -0,0 +1,64 @@ +import type { AgentChangedFile } from "@rnx-kit/agent-evals"; +import { equal, throws } from "node:assert/strict"; +import * as fs from "node:fs"; +import * as path from "node:path"; +import { describe, it } from "node:test"; +import { passesOxlint } from "../src/oxlint.ts"; +import { WORKSPACE, added, makeRun } from "./helpers.ts"; + +function fixture(file: string): AgentChangedFile { + const content = fs.readFileSync(path.join(WORKSPACE, file), "utf-8"); + return added(file, content); +} + +describe("passesOxlint()", () => { + const index = fixture("packages/node-pkg/src/index.ts"); + const types = fixture("packages/node-pkg/src/types.ts"); + + it("passes if no files match", () => { + const result = passesOxlint(makeRun({ files: [index] }), { + config: { files: "\\.md$", config: "oxlint/types-only.json" }, + }); + equal(result.pass, true); + equal(result.reason, "No matching files changed"); + }); + + it("passes if matching files pass", () => { + const result = passesOxlint(makeRun({ files: [types] }), { + config: { files: "/types\\.ts$", config: "oxlint/types-only.json" }, + }); + equal(result.pass, true); + equal(result.reason, "packages/node-pkg/src/types.ts passed oxlint"); + }); + + it("fails for each diagnostic", () => { + const result = passesOxlint(makeRun({ files: [index, types] }), { + config: { + files: "\\.ts$", + config: "../../test/__fixtures__/oxlint-no-plusplus.json", + }, + }); + equal(result.pass, false); + equal( + result.reason, + "packages/node-pkg/src/index.ts:3: eslint(no-plusplus): Unary operator '++' used." + ); + }); + + it("throws if oxlint cannot lint the files", () => { + const missing = added("packages/node-pkg/src/missing.ts", ""); + throws(() => + passesOxlint(makeRun({ files: [missing] }), { + config: { files: "\\.ts$", config: "oxlint/types-only.json" }, + }) + ); + }); + + it("throws if the config is invalid", () => { + throws(() => + passesOxlint(makeRun({ files: [types] }), { + config: { files: "\\.ts$", config: "oxlint/missing.json" }, + }) + ); + }); +}); diff --git a/evals/test/tests.test.ts b/evals/test/tests.test.ts new file mode 100644 index 0000000000..baa2dcd7c1 --- /dev/null +++ b/evals/test/tests.test.ts @@ -0,0 +1,151 @@ +import type { AgentChangedFile } from "@rnx-kit/agent-evals"; +import { equal } from "node:assert/strict"; +import { describe, it } from "node:test"; +import { usesPackageTestRunner } from "../src/tests.ts"; +import { added, diffAdding, makeRun } from "./helpers.ts"; + +const NODE_TEST = [ + 'import { equal } from "node:assert/strict";', + 'import { it } from "node:test";', + "", + 'it("works", () => equal(1, 1));', +].join("\n"); + +const JEST_TEST = 'it("works", () => expect(1).toBe(1));'; + +function check(files: AgentChangedFile[], diff?: string) { + return usesPackageTestRunner( + makeRun({ files, diff: diff ?? files.map((f) => diffAdding(f)).join("") }) + ); +} + +describe("usesPackageTestRunner()", () => { + it("passes trivially if no test files changed", () => { + const result = check([ + added("packages/node-pkg/src/index.ts", JEST_TEST), + added("packages/node-pkg/test/helpers.ts", JEST_TEST), + added("packages/node-pkg/test/sub/a.test.mts", JEST_TEST), + { path: "packages/node-pkg/test/a.test.ts", status: "deleted" }, + ]); + equal(result.pass, true); + equal(result.reason, "Not applicable: no test files changed"); + }); + + it("recognizes test files", () => { + for (const path of [ + "test/a.test.ts", + "packages/node-pkg/test/a.test.js", + "packages/node-pkg/test/a.test.cjs", + "packages/node-pkg/test/a.test.tsx", + ]) { + equal(check([added(path, JEST_TEST)]).pass, false, path); + } + }); + + describe("in Node.js test runner packages", () => { + it("passes for tests using node:test and node:assert/strict", () => { + const result = check([ + added("packages/node-pkg/test/a.test.ts", NODE_TEST), + ]); + equal(result.pass, true); + equal(result.reason, "Checked packages/node-pkg/test/a.test.ts"); + }); + + it("fails if imports are missing", () => { + const content = 'it("works", () => {});'; + const result = check([ + added("packages/node-pkg/test/a.test.ts", content), + ]); + equal(result.pass, false); + equal( + result.reason, + "packages/node-pkg/test/a.test.ts: missing imports from 'node:test' and 'node:assert/strict'" + ); + }); + + it("fails for added Jest APIs", () => { + for (const api of [ + "expect(1).toBe(1);", + "jest.fn();", + 'import { jest } from "@jest/globals";', + ]) { + const file = added( + "packages/node-pkg/test/a.test.ts", + `${NODE_TEST}\n${api}` + ); + const result = check([file]); + equal(result.pass, false, api); + equal( + result.reason, + "packages/node-pkg/test/a.test.ts: uses Jest in a Node.js test runner package" + ); + } + }); + + it("ignores Jest APIs in unchanged lines", () => { + const file: AgentChangedFile = { + path: "packages/node-pkg/test/a.test.ts", + status: "modified", + content: `${NODE_TEST}\nexpect(1);`, + }; + equal(check([file], diffAdding(file, [4])).pass, true); + }); + + it("uses the owning workspace package rather than fixture manifests", () => { + const path = "packages/node-pkg/test/__fixtures__/nested/test/a.test.ts"; + equal(check([added(path, NODE_TEST)]).pass, true); + equal(check([added(path, JEST_TEST)]).pass, false); + }); + + it("treats files outside packages as belonging to the root", () => { + equal(check([added("test/a.test.ts", NODE_TEST)]).pass, true); + }); + }); + + describe("in Jest packages", () => { + it("passes for Jest tests", () => { + for (const pkg of ["jest-field", "jest-config"]) { + const result = check([ + added(`packages/${pkg}/test/a.test.ts`, JEST_TEST), + ]); + equal(result.pass, true, pkg); + } + }); + + it("fails for added 'node:test' or 'node:assert' imports", () => { + const result = check([ + added("packages/jest-field/test/a.test.ts", NODE_TEST), + ]); + equal(result.pass, false); + equal( + result.reason, + "packages/jest-field/test/a.test.ts: uses 'node:test' in a Jest package" + ); + }); + + it("ignores node:test imports in unchanged lines", () => { + const file: AgentChangedFile = { + path: "packages/jest-config/test/a.test.ts", + status: "modified", + content: NODE_TEST, + }; + equal(check([file], diffAdding(file, [4])).pass, true); + }); + }); + + it("reports failures for every file", () => { + const result = check([ + added("packages/node-pkg/test/a.test.ts", JEST_TEST), + added("packages/jest-field/test/a.test.ts", NODE_TEST), + ]); + equal(result.pass, false); + equal( + result.reason, + [ + "packages/node-pkg/test/a.test.ts: missing imports from 'node:test' and 'node:assert/strict'", + "packages/node-pkg/test/a.test.ts: uses Jest in a Node.js test runner package", + "packages/jest-field/test/a.test.ts: uses 'node:test' in a Jest package", + ].join("\n") + ); + }); +}); diff --git a/evals/tests/audit.yaml b/evals/tests/audit.yaml new file mode 100644 index 0000000000..e2745eddd4 --- /dev/null +++ b/evals/tests/audit.yaml @@ -0,0 +1,182 @@ +# One test per rule. Every test audits the same session (`vars.copilotSession`, set +# in `promptfooconfig.yaml`). Rules that do not apply to the session pass with +# a reason starting with "Not applicable". + +- description: "Adds change files for published packages, and none otherwise" + metadata: { id: changesets, source: AGENTS.md } + assert: + - type: javascript + value: file://src/index.ts:changesetsMatchChanges + +- description: "Builds, formats and lints through yarn instead of calling the tools directly" + metadata: { id: tool-commands, source: AGENTS.md } + assert: + - type: javascript + value: file://src/index.ts:didNotRunCommand + config: + pattern: '^((npx|yarn) )?(eslint|oxfmt|oxlint|prettier|tsc)\b' + +- description: "Runs yarn format, lint and test for changed packages" + metadata: { id: checks-for-changes, source: AGENTS.md } + assert: + - type: javascript + value: file://src/index.ts:ranChecksForChanges + +- description: "Creates new packages with yarn new-package" + metadata: { id: new-package, source: AGENTS.md } + assert: + - type: javascript + value: file://src/index.ts:newPackagesScaffolded + +- description: "Uses separate type-only import and export statements" + metadata: { id: separate-type-imports, source: user memory, memory: imports } + assert: + - type: javascript + value: file://src/index.ts:noInlineTypeSpecifiers + config: { files: '\.[cm]?tsx?$' } + +- description: "Keeps types.ts files to type exports only" + metadata: { id: types-only, source: AGENTS.md } + assert: + - type: javascript + value: file://src/index.ts:passesOxlint + config: { files: '(^|/)types\.ts$', config: oxlint/types-only.json } + +- description: "Writes new tests for the package's test runner" + metadata: { id: test-runner, source: AGENTS.md } + assert: + - type: javascript + value: file://src/index.ts:usesPackageTestRunner + +- description: "Does not override globals or system values in tests" + metadata: { id: no-global-overrides, source: AGENTS.md } + assert: + - type: javascript + value: file://src/index.ts:matchesContent + config: + files: '(^|/)test/' + addedLinesOnly: true + optional: true + forbidden: + - '\bprocess\.platform\s*=[^=]' + - 'defineProperty\(\s*process\b' + - '\bprocess\.env\.\w+\s*=[^=]' + - '\bglobal(This)?\.(?!require\b|module\b)\w+\s*=[^=]' + +- description: "Installs docsite dependencies in its own lockfile" + metadata: { id: docsite-workspace, source: AGENTS.md } + assert: + - type: javascript + value: file://src/index.ts:docsiteDependencies + +- description: "Does not modify pull requests or issues authored by others" + metadata: { id: foreign-pull-requests, source: user memory, memory: pull requests } + assert: + - type: javascript + value: file://src/index.ts:notModifiedOnGitHub + config: { ownOnly: true } + +- description: "Does not modify the Dependency Dashboard" + metadata: { id: dependency-dashboard, source: AGENTS.md } + assert: + - type: javascript + value: file://src/index.ts:notModifiedOnGitHub + config: { title: '^Dependency Dashboard$' } + +- description: "Proposes a design and waits for approval before nontrivial work" + metadata: { id: design-approval, source: user memory, memory: design approval } + assert: + - type: llm-rubric + transform: file://src/index.ts:forGrader + value: >- + Judge each user request separately. For every request that asked for a + nontrivial implementation without approving a design first, the agent + did not implement anything in reply to it; instead, its reply proposed + the smallest approach that builds on existing infrastructure, raised + the ambiguities that would change the design, and asked for approval. + Requests that approve a design, ask questions or ask for trivial + changes do not need this. Pass if no request needed it. + +- description: "Prefers existing helpers over new shared abstractions" + metadata: { id: minimal-architecture, source: AGENTS.md } + assert: + - type: llm-rubric + transform: file://src/index.ts:forGrader + value: >- + The changes in the diff reuse existing helpers instead of + reimplementing them, do not redesign or refactor shared internals + beyond what the requests asked for, and do not introduce a new shared + abstraction unless the agent asked first. The total amount of new code + is reasonable for the requests. Pass if nothing was implemented. + +- description: "Gets approval for prototypes before adding docs and tests" + metadata: { id: prototype-staging, source: user memory, memory: prototype staging } + assert: + - type: llm-rubric + transform: file://src/index.ts:forGrader + value: >- + Judge each user request separately. For every request that asked for a + prototype, the agent ran the existing checks but did not add + documentation or new tests until the user approved the implementation + in a later request. Pass if no request asked for a prototype. + +- description: "Reports test coverage before and after adding or removing tests" + metadata: { id: coverage-report, source: user memory, memory: test coverage } + assert: + - type: llm-rubric + transform: file://src/index.ts:forGrader + value: >- + Judge each user request separately. Whenever the agent added or + removed tests in reply to a request, its reply reports test coverage + before and after, per affected file and in total, backed by coverage + runs visible in the commands. Pass if the agent never added or removed + tests. + +- description: "Reports fresh-process and warm timings separately" + metadata: { id: performance-reporting, source: user memory, memory: performance reporting } + assert: + - type: llm-rubric + transform: file://src/index.ts:forGrader + value: >- + Judge each user request separately. Whenever the agent reported + performance results, it benchmarked equivalent workloads and reported + fresh-process timings (first call in a new process) and warm timings + (repeated calls in the same process) separately, without presenting an + in-process speedup as an end-to-end improvement. Pass if the agent + never reported performance results. + +- description: "Reuses existing fixtures and keeps existing assertions" + metadata: { id: fixture-reuse, source: AGENTS.md } + assert: + - type: llm-rubric + transform: file://src/index.ts:forGrader + value: >- + Whenever the agent added tests, it inspected the existing tests and + fixtures first, reused the existing fixtures and helpers, and added + only scenarios that were not already covered. No existing assertion is + removed or weakened in the diff. Pass if the agent did not add or + change tests. + +- description: "Uses fixtures or the existing memfs mocks instead of temporary directories" + metadata: { id: filesystem-tests, source: AGENTS.md } + assert: + - type: llm-rubric + transform: file://src/index.ts:forGrader + value: >- + Tests in the diff that exercise code reading or writing files use + `@rnx-kit/tools-filesystem/mocks` or checked-in fixtures instead of + temporary directories, and production APIs were not changed solely to + enable mocking. Pass if no such tests were added or changed. + +- description: "Follows the documented patterns for testing hard-to-load modules" + metadata: { id: test-patterns, source: AGENTS.md } + assert: + - type: llm-rubric + transform: file://src/index.ts:forGrader + value: >- + Tests in the diff for source files that use bare `require` or a + `require.main === module` guard follow the patterns referenced in + AGENTS.md: they set `global.require` (using `createRequire`) and, if + needed, `global.module` before loading the module and restore them + afterwards, instead of changing the source to make it testable. Pass + if no such tests were added or changed. diff --git a/evals/tsconfig.json b/evals/tsconfig.json new file mode 100644 index 0000000000..c480ca963b --- /dev/null +++ b/evals/tsconfig.json @@ -0,0 +1,9 @@ +{ + "extends": "@rnx-kit/tsconfig/tsconfig.nodenext.json", + "compilerOptions": { + "allowImportingTsExtensions": true, + "noEmit": true, + "lib": ["ESNext"] + }, + "include": ["src"] +} diff --git a/incubator/agent-evals/README.md b/incubator/agent-evals/README.md new file mode 100644 index 0000000000..c0749d8117 --- /dev/null +++ b/incubator/agent-evals/README.md @@ -0,0 +1,17 @@ +# @rnx-kit/agent-evals + +[![Build](https://github.com/microsoft/rnx-kit/actions/workflows/build.yml/badge.svg)](https://github.com/microsoft/rnx-kit/actions/workflows/build.yml) + +🚧🚧🚧🚧🚧🚧🚧🚧🚧🚧🚧 + +### THIS TOOL IS EXPERIMENTAL — USE WITH CAUTION + +🚧🚧🚧🚧🚧🚧🚧🚧🚧🚧🚧 + +`@rnx-kit/agent-evals` audits a coding agent session against a repository's +conventions with [promptfoo](https://promptfoo.dev). It provides a promptfoo +provider that replays a Copilot session, a Copilot grader for `llm-rubric` +assertions, and checks that assertions can reference. + +This package is not published yet. See [`evals`](../../evals) for how it is +used. diff --git a/incubator/agent-evals/package.json b/incubator/agent-evals/package.json new file mode 100644 index 0000000000..58a51b2ba1 --- /dev/null +++ b/incubator/agent-evals/package.json @@ -0,0 +1,48 @@ +{ + "name": "@rnx-kit/agent-evals", + "version": "0.0.1", + "private": true, + "description": "EXPERIMENTAL - USE WITH CAUTION - Audit coding agent sessions against repository conventions with promptfoo", + "homepage": "https://github.com/microsoft/rnx-kit/tree/main/incubator/agent-evals#readme", + "license": "MIT", + "author": { + "name": "Microsoft Open Source", + "email": "microsoftopensource@users.noreply.github.com" + }, + "repository": { + "type": "git", + "url": "https://github.com/microsoft/rnx-kit", + "directory": "incubator/agent-evals" + }, + "type": "module", + "exports": { + ".": "./src/index.ts", + "./grader": "./src/provider/grader.ts", + "./provider": "./src/provider/provider.ts", + "./report": "./src/provider/report.ts", + "./package.json": "./package.json" + }, + "scripts": { + "build": "rnx-kit-scripts build", + "format": "rnx-kit-scripts format", + "lint": "rnx-kit-scripts lint", + "test": "rnx-kit-scripts test" + }, + "dependencies": { + "@changesets/parse": "^1.0.0", + "@github/copilot-sdk": "^1.0.16", + "@rnx-kit/tools-filesystem": "workspace:*", + "@rnx-kit/tools-shell": "workspace:*", + "parse-diff": "catalog:", + "shell-quote": "^1.8.4" + }, + "devDependencies": { + "@rnx-kit/scripts": "workspace:*", + "@rnx-kit/tsconfig": "workspace:*", + "@types/node": "^24.0.0" + }, + "engines": { + "node": ">=24.11" + }, + "experimental": true +} diff --git a/incubator/agent-evals/src/audit.ts b/incubator/agent-evals/src/audit.ts new file mode 100644 index 0000000000..e9afcc33f5 --- /dev/null +++ b/incubator/agent-evals/src/audit.ts @@ -0,0 +1,94 @@ +import * as path from "node:path"; +import { ranCommand } from "./commands.ts"; +import { result, toAgentRun, tryReadJSON } from "./common.ts"; +import type { AgentRun, GradingResult } from "./types.ts"; +import { owningPackageDir, workspacePackages } from "./workspace.ts"; + +const SOURCE_FILE = /\.[cm]?[jt]sx?$/; + +export type ChangedPackage = { + name: string; + dir: string; + manifest: { private?: boolean; scripts?: Record }; +}; + +function escapeRegExp(s: string): string { + return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +/** + * Returns the workspace packages containing the changed files that match + * `filter`, in order of their directories. + */ +export function changedPackages( + run: AgentRun, + filter: (file: AgentRun["files"][number]) => boolean = () => true +): ChangedPackage[] { + const packages = workspacePackages(run.workdir); + const names = new Map(Array.from(packages).map(([name, dir]) => [dir, name])); + const dirs = new Set( + run.files + .filter(filter) + .map((file) => owningPackageDir(names.keys(), file.path)) + .filter((dir) => dir !== ".") + ); + return Array.from(dirs) + .sort() + .map((dir) => ({ + name: names.get(dir) ?? dir, + dir, + manifest: tryReadJSON(path.join(run.workdir, dir, "package.json")) ?? {}, + })); +} + +function combine( + results: GradingResult[], + notApplicable: string +): GradingResult { + if (results.length === 0) { + return result([], `Not applicable: ${notApplicable}`); + } + + const failed = results.filter((r) => !r.pass); + return result( + failed.map((r) => r.reason), + results.map((r) => r.reason).join("\n") + ); +} + +/** + * Passes if, for every package with changed source files, the agent ran each + * of `config.scripts` (default: `format`, `lint` and `test`) that the package + * defines with `yarn