From 974c892d6ad5fa49715c6a5ff57f5139e9ac616f Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Tue, 27 Oct 2026 14:51:42 -0700 Subject: [PATCH 01/10] docs(testing): new tests extend existing cases and stay at one layer Co-Authored-By: Claude Opus 5.5 --- .coderabbit.config.ts | 2 +- .macroscope/check-run-agents/test-signal.md | 37 +++++++++++++++++++++ AGENTS.md | 6 ++-- docs/internals/testing.md | 8 +++-- 4 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 .macroscope/check-run-agents/test-signal.md diff --git a/.coderabbit.config.ts b/.coderabbit.config.ts index f22a4010df94f..0b7d0182e6df7 100644 --- a/.coderabbit.config.ts +++ b/.coderabbit.config.ts @@ -27,7 +27,7 @@ export default defineConfig((ctx) => ({ instructions: "Hold changed code to the rules in docs/internals/web-ui.md.", }, { - path: "**/*.test.{ts,tsx}", + path: "**/*.test.*", instructions: "Hold changed tests to the rules in docs/internals/testing.md. Request tests only as those rules allow.", }, diff --git a/.macroscope/check-run-agents/test-signal.md b/.macroscope/check-run-agents/test-signal.md new file mode 100644 index 0000000000000..391e11c6de37b --- /dev/null +++ b/.macroscope/check-run-agents/test-signal.md @@ -0,0 +1,37 @@ +--- +title: Test Signal +model: gpt-6-sol +effort: max +input: incremental +tools: + - browse_code + - modify_pr +# An explicit include overrides ignore.md, which skips test files for every other check. +include: + - "**/*.test.*" +exclude: + - ".repos/**" +requires: + - Check +maxBudgetPerRun: 2 +maxBudgetPerPR: 10 +conclusion: neutral +--- + +# Test signal review + +Review only the tests this pull request adds or grows. Tests it moves, renames, or deletes are not findings. The rules are in `docs/internals/testing.md` under `## What to test`; read that section first and hold new tests to it. + +## Checklist + +Flag a new or grown test when one of these is true: + +- It tests what types already guarantee, trivial logic, copy text, a smoke run, mock calls that mirror the implementation, callback wiring, or props rendered to static markup. +- An existing test or table in the same file already covers the behavior. The case belongs there as a row or an assertion. +- The same case is also tested at another layer, for example in a service test and its wrapper test, at each call site of one helper, or in web and mobile copies of the same logic. Keep one copy at the lowest layer that fails on `main`. + +## Reporting + +Post each finding as a brief inline comment on the new test. Name the rule and give one fix: fold the case into the named test or table, delete it, or keep the copy at the named layer and delete the others. Ask only for fewer or smaller tests, never for more coverage. + +When there are no findings, make the entire final response exactly `All clear` on one line with nothing else. diff --git a/AGENTS.md b/AGENTS.md index f990c3d36b706..f4516fb90b4b1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -102,11 +102,13 @@ An empty database is a bad test. Seed your worktree's `.t3` with a copy of real - While you iterate, run `vp test run `. For big files, add `-t ""`. - Use `vp run typecheck`, not `npx tsc`; it caches per package. Both run tsc-rs (`typescript` is the `@tsc-rs/typescript` drop-in). - Run `node -e` snippets from `apps/server` or the owning package. `effect` does not resolve from the repo root. -- Test meaningful logic or observable behavior. Do not render components to static markup to assert props or attributes, or add tests that merely assert callback wiring or mirror the implementation. +- Test behavior that can break, such as orchestration and event-sourcing, provider protocol translation, auth and permission guards, persistence and migrations, external-format parsing, race fixes, and the logic a bug fix changed. Leave out tests for what types already guarantee, trivial logic, copy text, smoke runs, mock-call mirrors, callback wiring, and props rendered to static markup. +- Extend before you add. When an existing test or table covers the behavior, add a row or an assertion there. Write a new `it` only when no case fits. +- Test at one layer: the lowest one that fails on `main`. Do not repeat the case in a wrapper test, at each call site, or in web and mobile copies of the same logic. - [Testing](docs/internals/testing.md) carries these same test rules, plus review guidance, for review bots. Change both together. - **Do not run repo-wide checks.** No `vp check`, no `vp run -r test`, no `vp run -r typecheck` unless I ask. CI owns the full suite. - Backend behavior changes ship with focused tests for that behavior. -- Proof for a bug fix depends on the bug. Logic, data, state, and protocol bugs get a failing test first (the fewest that fail on `main` for that reason, in the module's existing test file), then the fix. Purely visual bugs (spacing, colour, alignment, a layout shift) get before/after captures as their proof and no test: do not add UI tests for nitpicks, and do not ask for one in review when a capture already shows it. A server-only fix with no visible surface ships the test plus a short runtime transcript. +- Proof for a bug fix depends on the bug. Logic, data, state, and protocol bugs get a failing test first (the fewest that fail on `main` for that reason, in an existing case of the module's test file where one fits), then the fix. Purely visual bugs (spacing, colour, alignment, a layout shift) get before/after captures as their proof and no test: do not add UI tests for nitpicks, and do not ask for one in review when a capture already shows it. A server-only fix with no visible surface ships the test plus a short runtime transcript. - The server is event-sourced, and side effects run after the command commits. In tests, drain the effect worker (`OrchestrationEffectWorkerV2.drain`) or await the specific persisted event or `Deferred` that marks the milestone. Never wait on sleeps or polling. A test that needs a timeout to pass is wrong. Concretely: - `it.effect` runs on a `TestClock`; `Effect.sleep` only advances through `TestClock.adjust`. `it.live` is wall-clock and only for subprocess or network fixtures. - No `yieldNow` or `sleep` polling loops. Await the `Deferred`, take from the `Queue`, or drain the worker. diff --git a/docs/internals/testing.md b/docs/internals/testing.md index 6b04fad40dab6..5446a4b93f330 100644 --- a/docs/internals/testing.md +++ b/docs/internals/testing.md @@ -4,10 +4,12 @@ Rules for writing tests, and for asking for them in review. AGENTS.md carries th ## What to test -- Test meaningful logic or observable behavior. Do not render components to static markup to assert props or attributes, or add tests that merely assert callback wiring or mirror the implementation. -- In review, ask for a missing test only when it would catch a real regression. Do not ask for one assertion per constant, label, or `switch` case. +- Test behavior that can break, such as orchestration and event-sourcing, provider protocol translation, auth and permission guards, persistence and migrations, external-format parsing, race fixes, and the logic a bug fix changed. Leave out tests for what types already guarantee, trivial logic, copy text, smoke runs, mock-call mirrors, callback wiring, and props rendered to static markup. +- Extend before you add. When an existing test or table covers the behavior, add a row or an assertion there. Write a new `it` only when no case fits. +- Test at one layer: the lowest one that fails on `main`. Do not repeat the case in a wrapper test, at each call site, or in web and mobile copies of the same logic. +- In review, ask for a missing test only when it would catch a real regression. Do not ask for one assertion per constant, label, or `switch` case. Flag a new test that breaks the rules above, and name the fix: fold it into the existing case or table, delete it, or keep one layer. - Backend behavior changes ship with focused tests for that behavior. -- Proof for a bug fix depends on the bug. Logic, data, state, and protocol bugs get a failing test first (the fewest that fail on `main` for that reason, in the module's existing test file), then the fix. Purely visual bugs (spacing, colour, alignment, a layout shift) get before/after captures as their proof and no test: do not add UI tests for nitpicks, and do not ask for one in review when a capture already shows it. A server-only fix with no visible surface ships the test plus a short runtime transcript. +- Proof for a bug fix depends on the bug. Logic, data, state, and protocol bugs get a failing test first (the fewest that fail on `main` for that reason, in an existing case of the module's test file where one fits), then the fix. Purely visual bugs (spacing, colour, alignment, a layout shift) get before/after captures as their proof and no test: do not add UI tests for nitpicks, and do not ask for one in review when a capture already shows it. A server-only fix with no visible surface ships the test plus a short runtime transcript. ## Waiting for work From b6794b0bb0113199baaec0f3082a9cb2c676f28c Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Tue, 27 Oct 2026 16:09:36 -0700 Subject: [PATCH 02/10] chore(review): test signal check runs on a cheap model Co-Authored-By: Claude Opus 5.5 --- .macroscope/check-run-agents/test-signal.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.macroscope/check-run-agents/test-signal.md b/.macroscope/check-run-agents/test-signal.md index 391e11c6de37b..fc6f0ae1da1b9 100644 --- a/.macroscope/check-run-agents/test-signal.md +++ b/.macroscope/check-run-agents/test-signal.md @@ -1,7 +1,7 @@ --- title: Test Signal -model: gpt-6-sol -effort: max +model: gpt-6-1-terra +reasoning: low input: incremental tools: - browse_code @@ -13,8 +13,8 @@ exclude: - ".repos/**" requires: - Check -maxBudgetPerRun: 2 -maxBudgetPerPR: 10 +maxBudgetPerRun: 1 +maxBudgetPerPR: 5 conclusion: neutral --- From fa38a5537ec39b95a956efa83aa7f14444d6b4fd Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Tue, 27 Oct 2026 16:32:45 -0700 Subject: [PATCH 03/10] test(evals): test-signal check is cut to 88 words, measured on cheap models Co-Authored-By: Claude Opus 5.5 --- .macroscope/check-run-agents/test-signal.md | 12 +- evals/README.md | 9 +- evals/mcp-tool-use/run.ts | 29 +- evals/package.json | 1 + evals/shared/proxy.ts | 32 ++ evals/test-signal/README.md | 23 ++ evals/test-signal/cases.ts | 177 +++++++++++ evals/test-signal/grade.ts | 51 ++++ evals/test-signal/run.test.ts | 27 ++ evals/test-signal/run.ts | 314 ++++++++++++++++++++ 10 files changed, 632 insertions(+), 43 deletions(-) create mode 100644 evals/shared/proxy.ts create mode 100644 evals/test-signal/README.md create mode 100644 evals/test-signal/cases.ts create mode 100644 evals/test-signal/grade.ts create mode 100644 evals/test-signal/run.test.ts create mode 100644 evals/test-signal/run.ts diff --git a/.macroscope/check-run-agents/test-signal.md b/.macroscope/check-run-agents/test-signal.md index fc6f0ae1da1b9..5c5319aafb739 100644 --- a/.macroscope/check-run-agents/test-signal.md +++ b/.macroscope/check-run-agents/test-signal.md @@ -22,16 +22,6 @@ conclusion: neutral Review only the tests this pull request adds or grows. Tests it moves, renames, or deletes are not findings. The rules are in `docs/internals/testing.md` under `## What to test`; read that section first and hold new tests to it. -## Checklist - -Flag a new or grown test when one of these is true: - -- It tests what types already guarantee, trivial logic, copy text, a smoke run, mock calls that mirror the implementation, callback wiring, or props rendered to static markup. -- An existing test or table in the same file already covers the behavior. The case belongs there as a row or an assertion. -- The same case is also tested at another layer, for example in a service test and its wrapper test, at each call site of one helper, or in web and mobile copies of the same logic. Keep one copy at the lowest layer that fails on `main`. - -## Reporting - -Post each finding as a brief inline comment on the new test. Name the rule and give one fix: fold the case into the named test or table, delete it, or keep the copy at the named layer and delete the others. Ask only for fewer or smaller tests, never for more coverage. +Post each finding as a brief inline comment on the new test with one fix. Ask only for fewer or smaller tests, never for more coverage. When there are no findings, make the entire final response exactly `All clear` on one line with nothing else. diff --git a/evals/README.md b/evals/README.md index 22b7ece5f4655..a58291a54d11d 100644 --- a/evals/README.md +++ b/evals/README.md @@ -4,10 +4,11 @@ An eval runs a real model against T3 Code's real text or code and scores what th Write or run an eval when a change affects what a model sees or does: agent instructions, MCP tool descriptions and schemas, tool results, generated titles, or the guidance an agent gets for a feature such as Device Manager. -| Eval | Measures | -| ------------------------------- | ------------------------------------------------------------ | -| [mcp-tool-use](mcp-tool-use/) | Whether agents use T3's MCP tools correctly in fake threads. | -| [thread-titles](thread-titles/) | Whether generated thread titles name the right subject. | +| Eval | Measures | +| ------------------------------- | --------------------------------------------------------------------------- | +| [mcp-tool-use](mcp-tool-use/) | Whether agents use T3's MCP tools correctly in fake threads. | +| [thread-titles](thread-titles/) | Whether generated thread titles name the right subject. | +| [test-signal](test-signal/) | Whether a cheap model running the test-signal check flags low-signal tests. | ## Run an eval diff --git a/evals/mcp-tool-use/run.ts b/evals/mcp-tool-use/run.ts index 91b1f442348ac..b3e1815d30f98 100644 --- a/evals/mcp-tool-use/run.ts +++ b/evals/mcp-tool-use/run.ts @@ -50,6 +50,7 @@ import { CLAUDE_PROVIDER, makeClaudeQueryOptions, } from "../../apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts"; +import { proxyConfig } from "../shared/proxy.ts"; import { changeRequestsCreatedBy } from "../../apps/server/src/orchestration-v2/PullRequestSyncReactor.ts"; /** Agent models by id, with the display name a harness gives them. */ const MODELS = { @@ -1342,34 +1343,6 @@ const callCodex = Effect.fn("evaluateMcpToolUse.callCodex")(function* ( }; }); -// Read credentials into memory only. Python's TOML parser avoids another runner dependency. -const decodeProxyConfig = Schema.decodeUnknownSync( - Schema.Struct({ url: Schema.String, headers: Schema.Record(Schema.String, Schema.String) }), -); -const proxyConfig = () => - decodeProxyConfig( - JSON.parse( - NodeChildProcess.execFileSync( - "python3", - [ - "-c", - ` -import json, os, pathlib, tomllib -home = pathlib.Path.home() / '.codex' -p = tomllib.loads((home / 'config.toml').read_text())['model_providers']['vibeproxy'] -h = p.get('http_headers', {}).copy() -h.update({k: os.environ[v] for k,v in p.get('env_http_headers', {}).items()}) -key = os.environ[p['env_key']] if p.get('env_key') else p.get('experimental_bearer_token') -if not key and p.get('requires_openai_auth'): key = json.loads((home / 'auth.json').read_text()).get('OPENAI_API_KEY') -if key: h['Authorization'] = 'Bearer ' + key -print(json.dumps({'url': p['base_url'].rstrip('/'), 'headers': h})) -`, - ], - { encoding: "utf8" }, - ), - ), - ); - const postModel = Effect.fn("evaluateMcpToolUse.postModel")(function* ( apiKey: string, body: unknown, diff --git a/evals/package.json b/evals/package.json index cf4d6e955647e..681da5963142b 100644 --- a/evals/package.json +++ b/evals/package.json @@ -6,6 +6,7 @@ "mcp-tool-use": "node --env-file-if-exists=../.env mcp-tool-use/run.ts", "mcp-tool-use:trim-capture": "node mcp-tool-use/trim-capture.ts", "thread-titles": "node --env-file-if-exists=../.env thread-titles/run.ts", + "test-signal": "node --env-file-if-exists=../.env test-signal/run.ts", "typecheck": "tsc --noEmit", "test": "vp test run --config ../vite.config.ts --dir ." }, diff --git a/evals/shared/proxy.ts b/evals/shared/proxy.ts new file mode 100644 index 0000000000000..9cf38202025bc --- /dev/null +++ b/evals/shared/proxy.ts @@ -0,0 +1,32 @@ +// @effect-diagnostics nodeBuiltinImport:off +import * as NodeChildProcess from "node:child_process"; +import * as Schema from "effect/Schema"; + +// Read credentials into memory only. Python's TOML parser avoids another runner dependency. +const decodeProxyConfig = Schema.decodeUnknownSync( + Schema.Struct({ url: Schema.String, headers: Schema.Record(Schema.String, Schema.String) }), +); +/** Reads the `vibeproxy` CLIProxyAPI provider from `~/.codex/config.toml`. */ +export const proxyConfig = () => + decodeProxyConfig( + JSON.parse( + NodeChildProcess.execFileSync( + "python3", + [ + "-c", + ` +import json, os, pathlib, tomllib +home = pathlib.Path.home() / '.codex' +p = tomllib.loads((home / 'config.toml').read_text())['model_providers']['vibeproxy'] +h = p.get('http_headers', {}).copy() +h.update({k: os.environ[v] for k,v in p.get('env_http_headers', {}).items()}) +key = os.environ[p['env_key']] if p.get('env_key') else p.get('experimental_bearer_token') +if not key and p.get('requires_openai_auth'): key = json.loads((home / 'auth.json').read_text()).get('OPENAI_API_KEY') +if key: h['Authorization'] = 'Bearer ' + key +print(json.dumps({'url': p['base_url'].rstrip('/'), 'headers': h})) +`, + ], + { encoding: "utf8" }, + ), + ), + ); diff --git a/evals/test-signal/README.md b/evals/test-signal/README.md new file mode 100644 index 0000000000000..0b1969618af86 --- /dev/null +++ b/evals/test-signal/README.md @@ -0,0 +1,23 @@ +# Test signal + +Runs the Macroscope `test-signal` check ([`.macroscope/check-run-agents/test-signal.md`](../../.macroscope/check-run-agents/test-signal.md)) on real and synthetic test diffs, and checks that a cheap model flags low-signal tests and leaves good ones alone. Use it to cut the check to the fewest words that still work. + +The model gets the check body, the PR's test diff, `read_file` and `grep` at the PR head, and `post_comment`. The wrapper prompt around the body is a stand-in for Macroscope's, which is not public. + +## Run + +```sh +cd evals +pnpm test-signal --out runs/baseline +pnpm test-signal --out runs/cut --prompt /path/to/variant.md --case new-table-row +``` + +- Models go through the `vibeproxy` CLIProxyAPI provider in `~/.codex/config.toml`. Default models: `gpt-6.1-terra` and `gpt-6-luna`. Pass `--model` once per model to change them. +- Flags: `--prompt ` (a variant of the check, default the real one), `--case ` (repeatable), `--samples ` (default 2, and a 3rd when the first 2 disagree), `--concurrency ` (default 6), `--reasoning ` (default `low`, as the check runs). +- A bad case passes when a comment lands inside a target test. A guard case passes when the check posts nothing and replies exactly `All clear`. +- `summary.md` gives the prompt hash and word count, then per model: bad cases caught, guards clean, errors, and tokens. +- Cost: proxy quota only, about 0.5M tokens for a full run on both models. + +## Add a case + +Add an entry to `cases.ts`. A `real` case names a merged commit and its test paths. A `synthetic` case inserts one test after an anchor string in a real file. Bad cases list the titles of the tests to flag. diff --git a/evals/test-signal/cases.ts b/evals/test-signal/cases.ts new file mode 100644 index 0000000000000..2b4f48aa8a1f7 --- /dev/null +++ b/evals/test-signal/cases.ts @@ -0,0 +1,177 @@ +/** + * Pull request test diffs for the test-signal check. A `real` case replays test files from a + * merged PR. A `synthetic` case inserts one test into a real file at `rev`, for a pattern with no + * clean real example. + */ +export type Case = { + readonly id: string; + readonly kind: "bad" | "guard"; + readonly source: string; + readonly rev: string; + /** Titles of the tests the check must flag. Guard cases expect no comment and `All clear`. */ + readonly targets: ReadonlyArray<{ readonly path: string; readonly title: string }>; +} & ( + | { readonly type: "real"; readonly paths: ReadonlyArray } + | { + readonly type: "synthetic"; + readonly path: string; + readonly after: string; + readonly insert: string; + } +); + +const base = "cb7e3814d71c8fcdc5b803c660c29b7d7e39be96"; + +export const cases: ReadonlyArray = [ + { + id: "copy-text-at-every-call-site", + kind: "bad", + source: "#23999 checks the PR or MR label in 11 files", + type: "real", + rev: "218e5c09df", + paths: [ + "packages/client-runtime/src/state/gitActions.test.ts", + "apps/web/src/lib/composerContextRecords.test.ts", + "apps/mobile/src/lib/composerContext.test.ts", + ], + targets: [ + { + path: "packages/client-runtime/src/state/gitActions.test.ts", + title: "says PR for a GitHub ref", + }, + { + path: "packages/client-runtime/src/state/gitActions.test.ts", + title: "says MR for a GitLab ref", + }, + { + path: "packages/client-runtime/src/state/gitActions.test.ts", + title: "names the host's change request in the default-branch dialog", + }, + { + path: "apps/web/src/lib/composerContextRecords.test.ts", + title: "names the change request the way its host does", + }, + { + path: "apps/mobile/src/lib/composerContext.test.ts", + title: "describes the change request with its host's term", + }, + ], + }, + { + id: "same-case-in-wrapper-test", + kind: "bad", + source: "#22452 added the same two cases to threadAccess and OrchestratorMcpService", + type: "real", + rev: "b260a7cf69", + paths: [ + "apps/server/src/mcp/threadAccess.test.ts", + "apps/server/src/mcp/OrchestratorMcpService.test.ts", + ], + // Either copy counts: the check must notice the duplicate. Which copy it keeps is secondary. + targets: [ + { + path: "apps/server/src/mcp/threadAccess.test.ts", + title: "points a bare uuid at the mcp: thread it belongs to", + }, + { + path: "apps/server/src/mcp/threadAccess.test.ts", + title: "keeps the plain message for an id that matches nothing", + }, + { + path: "apps/server/src/mcp/OrchestratorMcpService.test.ts", + title: "points a bare uuid at the mcp: thread it belongs to", + }, + { + path: "apps/server/src/mcp/OrchestratorMcpService.test.ts", + title: "keeps the plain message for an id that matches nothing", + }, + ], + }, + { + id: "new-it-for-a-table-row", + kind: "bad", + source: "Synthetic: a new `it` next to the table that already covers it", + type: "synthetic", + rev: base, + path: "packages/shared/src/markdownLinks.test.ts", + after: " expect(parseMarkdownFileLink(href)?.path).toBe(path);\n });\n", + insert: + '\n it("recognizes a Windows path with forward slashes as a file", () => {\n expect(parseMarkdownFileLink("C:/repo/src/app.ts")?.path).toBe("C:/repo/src/app.ts");\n });\n', + targets: [ + { + path: "packages/shared/src/markdownLinks.test.ts", + title: "recognizes a Windows path with forward slashes as a file", + }, + ], + }, + { + id: "mock-call-mirror", + kind: "bad", + source: "Synthetic: asserts the mock call the code makes, already covered above it", + type: "synthetic", + rev: base, + path: "packages/client-runtime/src/state/threadRequests.test.ts", + after: " expect(send).toHaveBeenCalledOnce();\n });\n", + insert: + '\n it("calls respond with decline", async () => {\n const respond = vi.fn(async () => AsyncResult.success(undefined));\n await declinePendingApprovals([approval], respond);\n expect(respond).toHaveBeenCalledWith(requestId, "decline");\n expect(respond).toHaveBeenCalledTimes(1);\n });\n', + targets: [ + { + path: "packages/client-runtime/src/state/threadRequests.test.ts", + title: "calls respond with decline", + }, + ], + }, + { + id: "type-guaranteed", + kind: "bad", + source: "Synthetic: checks a return type the signature already declares", + type: "synthetic", + rev: base, + path: "packages/shared/src/markdownLinks.test.ts", + after: " expect(isMarkdownFileLinkLabel(label, href)).toBe(expected);\n });\n", + insert: + '\n it("returns a boolean", () => {\n expect(typeof isMarkdownFileLinkLabel("a.ts", "/repo/a.ts")).toBe("boolean");\n });\n', + targets: [{ path: "packages/shared/src/markdownLinks.test.ts", title: "returns a boolean" }], + }, + { + id: "deadlock-race-fix", + kind: "guard", + source: "#22094 run metrics read lineage inside the transaction so a writer cannot deadlock", + type: "real", + rev: "f8e4834861", + paths: ["apps/server/src/orchestration-v2/RunMetrics.test.ts"], + targets: [], + }, + { + id: "clone-retry-fix", + kind: "guard", + source: "#23170 clones retry over HTTPS when the SSH key is refused", + type: "real", + rev: "a49d18cf3b", + paths: ["apps/server/src/sourceControl/SourceControlRepositoryService.test.ts"], + targets: [], + }, + { + id: "deleted-tests", + kind: "guard", + source: "#22592 pruned low-value tests", + type: "real", + rev: "a2f4df0412", + paths: [ + "apps/server/src/workspace/WorkspaceEntries.test.ts", + "apps/server/src/workspace/WorkspacePaths.test.ts", + ], + targets: [], + }, + { + id: "new-table-row", + kind: "guard", + source: "Synthetic: the right way to add the Windows path case", + type: "synthetic", + rev: base, + path: "packages/shared/src/markdownLinks.test.ts", + after: ' ["file://localhost/home/me/notes.md", "/home/me/notes.md"],\n', + insert: ' ["C:/repo/src/app.ts", "C:/repo/src/app.ts"],\n', + targets: [], + }, +]; diff --git a/evals/test-signal/grade.ts b/evals/test-signal/grade.ts new file mode 100644 index 0000000000000..b4af4cef760b9 --- /dev/null +++ b/evals/test-signal/grade.ts @@ -0,0 +1,51 @@ +export type Comment = { readonly path: string; readonly line: number; readonly body: string }; + +const testStart = /^\s*(it|test|describe)(\.\w+)*\(/; + +/** 1-based line range of the test whose title is `title`, up to the next test or describe. */ +export const testRange = (source: string, title: string) => { + const lines = source.split("\n"); + const start = lines.findIndex( + (line) => line.includes(`"${title}"`) || line.includes(`'${title}'`), + ); + if (start < 0) return undefined; + const next = lines.findIndex((line, index) => index > start && testStart.test(line)); + return { start: start + 1, end: next < 0 ? lines.length : next }; +}; + +// Models often anchor a comment on the blank line or brace just above the test. +const slack = 2; + +/** + * A bad case passes when a comment lands inside a target test. A guard case passes when the + * check posts nothing and ends with exactly `All clear`, as Macroscope requires. + */ +export const grade = (input: { + readonly kind: "bad" | "guard"; + readonly targets: ReadonlyArray<{ + readonly path: string; + readonly start: number; + readonly end: number; + }>; + readonly comments: ReadonlyArray; + readonly final: string; +}) => { + if (input.kind === "guard") { + if (input.comments.length > 0) + return { pass: false, reason: `${input.comments.length} comment(s)` }; + if (input.final.trim() !== "All clear") + return { pass: false, reason: "final reply is not `All clear`" }; + return { pass: true, reason: "all clear" }; + } + const hit = input.comments.some((comment) => + input.targets.some( + (target) => + comment.path === target.path && + comment.line >= target.start - slack && + comment.line <= target.end, + ), + ); + return hit + ? { pass: true, reason: "flagged a target" } + : { pass: false, reason: `missed (${input.comments.length} comment(s) elsewhere)` }; +}; diff --git a/evals/test-signal/run.test.ts b/evals/test-signal/run.test.ts new file mode 100644 index 0000000000000..22c0bb45779f4 --- /dev/null +++ b/evals/test-signal/run.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vite-plus/test"; +import { grade, testRange } from "./grade.ts"; + +const source = [ + 'describe("x", () => {', + ' it("first", () => {', + " expect(1).toBe(1);", + " });", + "", + ' it("second", () => {});', + "});", +].join("\n"); + +describe("grade", () => { + const target = { path: "a.test.ts", ...testRange(source, "first")! }; + const comment = (line: number) => ({ path: "a.test.ts", line, body: "fold it" }); + + it.each([ + ["bad, comment inside the target", "bad", [comment(3)], "", true], + ["bad, comment on the next test", "bad", [comment(6)], "", false], + ["guard, nothing posted", "guard", [], "All clear", true], + ["guard, nothing posted but extra text", "guard", [], "All clear. Nice tests.", false], + ["guard, a comment", "guard", [comment(3)], "All clear", false], + ] as const)("%s", (_name, kind, comments, final, pass) => { + expect(grade({ kind, targets: [target], comments, final }).pass).toBe(pass); + }); +}); diff --git a/evals/test-signal/run.ts b/evals/test-signal/run.ts new file mode 100644 index 0000000000000..500b281ea4cbd --- /dev/null +++ b/evals/test-signal/run.ts @@ -0,0 +1,314 @@ +#!/usr/bin/env node +// @effect-diagnostics nodeBuiltinImport:off +import * as NodeChildProcess from "node:child_process"; +import * as NodeCrypto from "node:crypto"; +import * as NodeFS from "node:fs"; +import * as NodeOS from "node:os"; +import * as NodePath from "node:path"; +import * as NodeUtil from "node:util"; +import * as Effect from "effect/Effect"; +import * as Schedule from "effect/Schedule"; +import * as Schema from "effect/Schema"; +import { FetchHttpClient, HttpClient, HttpClientRequest } from "effect/http"; +import { proxyConfig } from "../shared/proxy.ts"; +import { cases, type Case } from "./cases.ts"; +import { grade, testRange, type Comment } from "./grade.ts"; + +const { values } = NodeUtil.parseArgs({ + options: { + out: { type: "string" }, + model: { type: "string", multiple: true }, + prompt: { type: "string" }, + case: { type: "string", multiple: true }, + samples: { type: "string", default: "2" }, + concurrency: { type: "string", default: "6" }, + reasoning: { type: "string", default: "low" }, + }, +}); +if (!values.out) throw new Error("Use --out [--model ] [--prompt ]."); +const outDirectory = values.out; +const models = values.model ?? ["gpt-6.1-terra", "gpt-6-luna"]; +const repo = NodePath.resolve(import.meta.dirname, "../.."); +const checkFile = + values.prompt ?? NodePath.join(repo, ".macroscope/check-run-agents/test-signal.md"); +// Macroscope gives the model the body; the front matter only configures the run. +const instructions = NodeFS.readFileSync(checkFile, "utf8") + .replace(/^---\n[\s\S]*?\n---\n/, "") + .trim(); +const words = instructions.split(/\s+/).length; +const promptHash = NodeCrypto.createHash("sha256").update(instructions).digest("hex").slice(0, 12); +const selected = values.case ? cases.filter((entry) => values.case!.includes(entry.id)) : cases; +const proxy = proxyConfig(); + +const git = (args: ReadonlyArray) => { + const result = NodeChildProcess.spawnSync("git", args, { + cwd: repo, + encoding: "utf8", + maxBuffer: 1 << 26, + }); + return result.status === 0 ? result.stdout : undefined; +}; + +/** The case's PR head as files the model can read, plus the diff it reviews. */ +const world = (entry: Case) => { + const files = new Map(); + if (entry.type === "real") { + const diff = git(["diff", `${entry.rev}~1`, entry.rev, "--", ...entry.paths]) ?? ""; + return { diff, files }; + } + const before = git(["show", `${entry.rev}:${entry.path}`]); + if (!before?.includes(entry.after)) throw new Error(`${entry.id}: anchor not found.`); + const after = before.replace(entry.after, entry.after + entry.insert); + files.set(entry.path, after); + const directory = NodeFS.mkdtempSync(NodePath.join(NodeOS.tmpdir(), "t3-test-signal-")); + NodeFS.writeFileSync(NodePath.join(directory, "before"), before); + NodeFS.writeFileSync(NodePath.join(directory, "after"), after); + const raw = NodeChildProcess.spawnSync("git", ["diff", "--no-index", "before", "after"], { + cwd: directory, + encoding: "utf8", + }).stdout; + NodeFS.rmSync(directory, { recursive: true }); + const diff = raw + .replace("diff --git a/before b/after", `diff --git a/${entry.path} b/${entry.path}`) + .replace("--- a/before", `--- a/${entry.path}`) + .replace("+++ b/after", `+++ b/${entry.path}`); + return { diff, files }; +}; + +// The check reads the rules at the PR head, where they always exist. Old case commits predate +// them, so serve the rules from this checkout. +const ruleFiles = new Set(["docs/internals/testing.md", "AGENTS.md"]); +const readAt = (entry: Case, files: Map, path: string) => + ruleFiles.has(path) + ? NodeFS.readFileSync(NodePath.join(repo, path), "utf8") + : (files.get(path) ?? git(["show", `${entry.rev}:${path}`])); + +const tools = [ + { + type: "function", + name: "read_file", + description: "Read a file at the pull request head, with line numbers.", + parameters: { + type: "object", + properties: { + path: { type: "string" }, + start_line: { type: "integer" }, + end_line: { type: "integer" }, + }, + required: ["path"], + }, + }, + { + type: "function", + name: "grep", + description: + "Search the repository at the pull request head with an extended regular expression.", + parameters: { + type: "object", + properties: { pattern: { type: "string" }, path: { type: "string" } }, + required: ["pattern"], + }, + }, + { + type: "function", + name: "post_comment", + description: "Post an inline review comment on a line of the pull request head.", + parameters: { + type: "object", + properties: { path: { type: "string" }, line: { type: "integer" }, body: { type: "string" } }, + required: ["path", "line", "body"], + }, + }, +]; + +const system = (body: string) => + [ + "You are a Macroscope Check Run Agent reviewing a GitHub pull request.", + "Follow the check instructions below. Read the repository at the pull request head with read_file and grep.", + "Leave inline review comments with post_comment. Then give your final response.", + "", + "", + body, + "", + ].join("\n"); + +const decodeResponse = Schema.decodeUnknownSync( + Schema.fromJsonString( + Schema.Struct({ + output: Schema.Array(Schema.Record(Schema.String, Schema.Unknown)), + usage: Schema.optional(Schema.Struct({ total_tokens: Schema.optional(Schema.Number) })), + }), + ), +); +const decodeArguments = Schema.decodeUnknownSync( + Schema.fromJsonString(Schema.Record(Schema.String, Schema.Unknown)), +); +const encodeRow = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown)); + +const post = Effect.fn("evaluateTestSignal.post")(function* (body: unknown) { + const client = (yield* HttpClient.HttpClient).pipe( + HttpClient.retryTransient({ + retryOn: "errors-and-responses", + schedule: Schedule.exponential("2 seconds"), + times: 4, + }), + ); + const response = yield* HttpClientRequest.post(`${proxy.url}/responses`).pipe( + HttpClientRequest.setHeaders(proxy.headers), + HttpClientRequest.bodyJson(body), + Effect.flatMap(client.execute), + ); + const text = yield* response.text; + if (response.status !== 200) + return yield* Effect.die(new Error(`Model API ${response.status}: ${text}`)); + return decodeResponse(text); +}); + +const runSample = Effect.fn("evaluateTestSignal.sample")(function* (entry: Case, model: string) { + const { diff, files } = world(entry); + const comments: Array = []; + const input: Array = [ + { + role: "user", + content: `Pull request diff for the files this check includes:\n\n\`\`\`diff\n${diff}\`\`\``, + }, + ]; + let final = ""; + let tokens = 0; + for (let step = 0; step < 16; step++) { + const response = yield* post({ + model, + instructions: system(instructions), + input, + tools, + reasoning: { effort: values.reasoning }, + store: false, + include: ["reasoning.encrypted_content"], + }); + tokens += response.usage?.total_tokens ?? 0; + input.push(...response.output); + const calls = response.output.filter((item) => item.type === "function_call"); + final = response.output + .filter((item) => item.type === "message") + .flatMap((item) => (item.content as Array<{ text?: string }>).map((part) => part.text ?? "")) + .join(""); + if (calls.length === 0) break; + for (const call of calls) { + const args = decodeArguments(String(call.arguments)); + let output: string; + if (call.name === "post_comment") { + comments.push({ + path: String(args.path), + line: Number(args.line), + body: String(args.body), + }); + output = "Posted."; + } else if (call.name === "read_file") { + const text = readAt(entry, files, String(args.path)); + const lines = text?.split("\n") ?? []; + const start = Math.max(1, Number(args.start_line ?? 1)); + const end = Math.min(lines.length, Number(args.end_line ?? start + 399)); + output = + text === undefined + ? "File not found." + : lines + .slice(start - 1, end) + .map((line, index) => `${start + index}: ${line}`) + .join("\n"); + } else { + const found = git([ + "grep", + "-n", + "-E", + String(args.pattern), + entry.rev, + "--", + String(args.path ?? "."), + ]); + output = + found?.split("\n").slice(0, 80).join("\n").replaceAll(`${entry.rev}:`, "") || + "No matches."; + } + input.push({ type: "function_call_output", call_id: call.call_id, output }); + } + } + const targets = entry.targets.flatMap((target) => { + const range = testRange(readAt(entry, files, target.path) ?? "", target.title); + if (!range) throw new Error(`${entry.id}: target "${target.title}" not found.`); + return [{ path: target.path, ...range }]; + }); + return { ...grade({ kind: entry.kind, targets, comments, final }), comments, final, tokens }; +}); + +const sampleSafely = (entry: Case, model: string, sample: number) => + runSample(entry, model).pipe( + Effect.map((result) => ({ case: entry.id, kind: entry.kind, model, sample, ...result })), + Effect.catchCause((cause) => + Effect.succeed({ case: entry.id, kind: entry.kind, model, sample, error: String(cause) }), + ), + ); + +type Row = Effect.Success>; +const isGraded = (row: Row) => "pass" in row; +const passed = (row: Row) => "pass" in row && row.pass; + +const summary = await Effect.runPromise( + Effect.gen(function* () { + NodeFS.mkdirSync(outDirectory, { recursive: true }); + const samples = Number(values.samples); + const pairs = models.flatMap((model) => selected.map((entry) => ({ model, entry }))); + const rows: Array = (yield* Effect.forEach( + pairs.flatMap(({ model, entry }) => + Array.from({ length: samples }, (_, n) => ({ model, entry, n })), + ), + ({ model, entry, n }) => sampleSafely(entry, model, n), + { concurrency: Number(values.concurrency) }, + )).slice(); + // A third sample settles a split. + const splits = pairs.filter(({ model, entry }) => { + const passes = rows.filter( + (row) => row.model === model && row.case === entry.id && isGraded(row), + ); + return passes.length === 2 && passes.some(passed) && !passes.every(passed); + }); + rows.push( + ...(yield* Effect.forEach(splits, ({ model, entry }) => sampleSafely(entry, model, samples), { + concurrency: Number(values.concurrency), + })), + ); + NodeFS.writeFileSync( + NodePath.join(outDirectory, "results.ndjson"), + rows.map((row) => encodeRow(row)).join("\n") + "\n", + ); + + const lines = [`prompt ${promptHash}, ${words} words, ${checkFile}`, ""]; + lines.push("| model | bad caught | guards clean | errors | tokens |", "|---|---|---|---|---|"); + for (const model of models) { + const verdicts = selected.map((entry) => { + const own = rows.filter((row) => row.model === model && row.case === entry.id); + const graded = own.filter(isGraded); + const passes = graded.filter(passed).length; + return { + entry, + pass: graded.length > 0 && passes > graded.length / 2, + errors: own.length - graded.length, + }; + }); + const count = (kind: Case["kind"]) => { + const own = verdicts.filter((verdict) => verdict.entry.kind === kind); + return `${own.filter((verdict) => verdict.pass).length}/${own.length}`; + }; + const tokens = rows + .filter((row) => row.model === model && "tokens" in row) + .reduce((sum, row) => sum + ("tokens" in row ? row.tokens : 0), 0); + const errors = verdicts.reduce((sum, verdict) => sum + verdict.errors, 0); + lines.push(`| ${model} | ${count("bad")} | ${count("guard")} | ${errors} | ${tokens} |`); + const failed = verdicts.filter((verdict) => !verdict.pass).map((verdict) => verdict.entry.id); + if (failed.length > 0) lines.push(`| failed: ${failed.join(", ")} | | | | |`); + } + const report = lines.join("\n") + "\n"; + NodeFS.writeFileSync(NodePath.join(outDirectory, "summary.md"), report); + return report; + }).pipe(Effect.provide(FetchHttpClient.layer)), +); +process.stdout.write(summary); From c285bf335e3d7055c29a14526fe3c5b119d345e9 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Tue, 27 Oct 2026 16:35:58 -0700 Subject: [PATCH 04/10] fix(evals): test-signal grep sees served files, and an empty case diff fails Co-Authored-By: Claude Opus 5.5 --- evals/test-signal/run.ts | 38 +++++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/evals/test-signal/run.ts b/evals/test-signal/run.ts index 500b281ea4cbd..ce843b7851ce8 100644 --- a/evals/test-signal/run.ts +++ b/evals/test-signal/run.ts @@ -53,7 +53,9 @@ const git = (args: ReadonlyArray) => { const world = (entry: Case) => { const files = new Map(); if (entry.type === "real") { - const diff = git(["diff", `${entry.rev}~1`, entry.rev, "--", ...entry.paths]) ?? ""; + const diff = git(["diff", `${entry.rev}~1`, entry.rev, "--", ...entry.paths]); + // An empty diff would grade a guard case as a clean pass. + if (!diff) throw new Error(`${entry.id}: no diff at ${entry.rev}.`); return { diff, files }; } const before = git(["show", `${entry.rev}:${entry.path}`]); @@ -83,6 +85,27 @@ const readAt = (entry: Case, files: Map, path: string) => ? NodeFS.readFileSync(NodePath.join(repo, path), "utf8") : (files.get(path) ?? git(["show", `${entry.rev}:${path}`])); +/** `git grep` at `rev`, with overlay and rule files searched in their served form instead. */ +const grepAt = (entry: Case, files: Map, pattern: string, scope: string) => { + const served = new Map(files); + for (const path of ruleFiles) served.set(path, readAt(entry, files, path) ?? ""); + const inScope = (path: string) => scope === "" || scope === "." || path.startsWith(scope); + const fromRev = (git(["grep", "-n", "-E", pattern, entry.rev, "--", scope || "."]) ?? "") + .split("\n") + .filter((line) => line !== "") + .map((line) => line.replace(`${entry.rev}:`, "")) + .filter((line) => !served.has(line.slice(0, line.indexOf(":")))); + const regex = new RegExp(pattern); + const fromServed = [...served] + .filter(([path]) => inScope(path)) + .flatMap(([path, text]) => + text + .split("\n") + .flatMap((line, index) => (regex.test(line) ? [`${path}:${index + 1}:${line}`] : [])), + ); + return [...fromServed, ...fromRev].slice(0, 80).join("\n") || "No matches."; +}; + const tools = [ { type: "function", @@ -216,18 +239,7 @@ const runSample = Effect.fn("evaluateTestSignal.sample")(function* (entry: Case, .map((line, index) => `${start + index}: ${line}`) .join("\n"); } else { - const found = git([ - "grep", - "-n", - "-E", - String(args.pattern), - entry.rev, - "--", - String(args.path ?? "."), - ]); - output = - found?.split("\n").slice(0, 80).join("\n").replaceAll(`${entry.rev}:`, "") || - "No matches."; + output = grepAt(entry, files, String(args.pattern), String(args.path ?? "")); } input.push({ type: "function_call_output", call_id: call.call_id, output }); } From 443664241c76f73b79605330fb5e68a941f2e8be Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Tue, 27 Oct 2026 16:37:31 -0700 Subject: [PATCH 05/10] chore(evals): say why the eval scripts use node builtins Co-Authored-By: Claude Opus 5.5 --- evals/shared/proxy.ts | 1 + evals/test-signal/run.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/evals/shared/proxy.ts b/evals/shared/proxy.ts index 9cf38202025bc..ede72be8fdab2 100644 --- a/evals/shared/proxy.ts +++ b/evals/shared/proxy.ts @@ -1,3 +1,4 @@ +// Eval-only helper: it reads the local Codex config through python3 at startup. // @effect-diagnostics nodeBuiltinImport:off import * as NodeChildProcess from "node:child_process"; import * as Schema from "effect/Schema"; diff --git a/evals/test-signal/run.ts b/evals/test-signal/run.ts index ce843b7851ce8..2f21569f7c869 100644 --- a/evals/test-signal/run.ts +++ b/evals/test-signal/run.ts @@ -1,4 +1,5 @@ #!/usr/bin/env node +// An eval script, not server code: it shells out to git and reads files directly. // @effect-diagnostics nodeBuiltinImport:off import * as NodeChildProcess from "node:child_process"; import * as NodeCrypto from "node:crypto"; From 919fcd9909b337d3c2f9ad6417be8ea7f3e05143 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Tue, 27 Oct 2026 16:39:51 -0700 Subject: [PATCH 06/10] fix(evals): a test-signal run error counts as a failed sample Co-Authored-By: Claude Opus 5.5 --- evals/test-signal/run.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/evals/test-signal/run.ts b/evals/test-signal/run.ts index 2f21569f7c869..ad41ed35d25a2 100644 --- a/evals/test-signal/run.ts +++ b/evals/test-signal/run.ts @@ -279,10 +279,8 @@ const summary = await Effect.runPromise( )).slice(); // A third sample settles a split. const splits = pairs.filter(({ model, entry }) => { - const passes = rows.filter( - (row) => row.model === model && row.case === entry.id && isGraded(row), - ); - return passes.length === 2 && passes.some(passed) && !passes.every(passed); + const own = rows.filter((row) => row.model === model && row.case === entry.id); + return own.length === 2 && own.some(passed) && !own.every(passed); }); rows.push( ...(yield* Effect.forEach(splits, ({ model, entry }) => sampleSafely(entry, model, samples), { @@ -299,11 +297,12 @@ const summary = await Effect.runPromise( for (const model of models) { const verdicts = selected.map((entry) => { const own = rows.filter((row) => row.model === model && row.case === entry.id); + // A run error counts as a failed sample, so one pass and one error is not a majority. const graded = own.filter(isGraded); - const passes = graded.filter(passed).length; + const passes = own.filter(passed).length; return { entry, - pass: graded.length > 0 && passes > graded.length / 2, + pass: passes > own.length / 2, errors: own.length - graded.length, }; }); From a141c0aafd1773c96544c26e75b6f25e0ac33cd1 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Tue, 27 Oct 2026 16:41:56 -0700 Subject: [PATCH 07/10] fix(evals): test-signal grep matches POSIX patterns and glob scopes in served files Co-Authored-By: Claude Opus 5.5 --- evals/test-signal/run.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/evals/test-signal/run.ts b/evals/test-signal/run.ts index ad41ed35d25a2..a3859d94bf4e5 100644 --- a/evals/test-signal/run.ts +++ b/evals/test-signal/run.ts @@ -90,19 +90,24 @@ const readAt = (entry: Case, files: Map, path: string) => const grepAt = (entry: Case, files: Map, pattern: string, scope: string) => { const served = new Map(files); for (const path of ruleFiles) served.set(path, readAt(entry, files, path) ?? ""); - const inScope = (path: string) => scope === "" || scope === "." || path.startsWith(scope); + // Match scopes and patterns the way `git grep` does: pathspec globs and POSIX EREs. + const inScope = (path: string) => + scope === "" || scope === "." || path.startsWith(scope) || NodePath.matchesGlob(path, scope); const fromRev = (git(["grep", "-n", "-E", pattern, entry.rev, "--", scope || "."]) ?? "") .split("\n") .filter((line) => line !== "") .map((line) => line.replace(`${entry.rev}:`, "")) .filter((line) => !served.has(line.slice(0, line.indexOf(":")))); - const regex = new RegExp(pattern); const fromServed = [...served] .filter(([path]) => inScope(path)) .flatMap(([path, text]) => - text - .split("\n") - .flatMap((line, index) => (regex.test(line) ? [`${path}:${index + 1}:${line}`] : [])), + NodeChildProcess.spawnSync("grep", ["-n", "-E", "--", pattern], { + input: text, + encoding: "utf8", + }) + .stdout.split("\n") + .filter((line) => line !== "") + .map((line) => `${path}:${line}`), ); return [...fromServed, ...fromRev].slice(0, 80).join("\n") || "No matches."; }; From ffd4897a1037292c8d84ca865ab3899774be7dcf Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Tue, 27 Oct 2026 16:45:59 -0700 Subject: [PATCH 08/10] fix(evals): test-signal grep scopes follow git pathspecs Co-Authored-By: Claude Opus 5.5 --- evals/test-signal/run.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/evals/test-signal/run.ts b/evals/test-signal/run.ts index a3859d94bf4e5..4f3a162ee9a84 100644 --- a/evals/test-signal/run.ts +++ b/evals/test-signal/run.ts @@ -90,9 +90,9 @@ const readAt = (entry: Case, files: Map, path: string) => const grepAt = (entry: Case, files: Map, pattern: string, scope: string) => { const served = new Map(files); for (const path of ruleFiles) served.set(path, readAt(entry, files, path) ?? ""); - // Match scopes and patterns the way `git grep` does: pathspec globs and POSIX EREs. - const inScope = (path: string) => - scope === "" || scope === "." || path.startsWith(scope) || NodePath.matchesGlob(path, scope); + // Served files also exist in this checkout, so git applies its own pathspec rules to the scope. + const matched = new Set((git(["ls-files", "--", scope || "."]) ?? "").split("\n")); + const inScope = (path: string) => matched.has(path); const fromRev = (git(["grep", "-n", "-E", pattern, entry.rev, "--", scope || "."]) ?? "") .split("\n") .filter((line) => line !== "") From a98d17738a4e7dfdfa36bfa5f08976bb1acd1e5e Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Tue, 27 Oct 2026 16:50:16 -0700 Subject: [PATCH 09/10] fix(evals): test-signal grep shows the error for a bad pattern Co-Authored-By: Claude Opus 5.5 --- evals/test-signal/run.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/evals/test-signal/run.ts b/evals/test-signal/run.ts index 4f3a162ee9a84..81ec39b55e3d0 100644 --- a/evals/test-signal/run.ts +++ b/evals/test-signal/run.ts @@ -88,6 +88,12 @@ const readAt = (entry: Case, files: Map, path: string) => /** `git grep` at `rev`, with overlay and rule files searched in their served form instead. */ const grepAt = (entry: Case, files: Map, pattern: string, scope: string) => { + // Status 2 is a bad pattern. Show grep's message, as a real tool would, so the model can fix it. + const check = NodeChildProcess.spawnSync("grep", ["-E", "--", pattern], { + input: "", + encoding: "utf8", + }); + if (check.status !== 0 && check.status !== 1) return check.stderr.trim() || "grep failed."; const served = new Map(files); for (const path of ruleFiles) served.set(path, readAt(entry, files, path) ?? ""); // Served files also exist in this checkout, so git applies its own pathspec rules to the scope. From 8cac3838f72bf6d655512c5c06ae6101c70dc090 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Tue, 27 Oct 2026 16:53:37 -0700 Subject: [PATCH 10/10] fix(evals): test-signal grep accepts patterns that start with a dash Co-Authored-By: Claude Opus 5.5 --- evals/test-signal/run.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evals/test-signal/run.ts b/evals/test-signal/run.ts index 81ec39b55e3d0..855166c4e12e6 100644 --- a/evals/test-signal/run.ts +++ b/evals/test-signal/run.ts @@ -99,7 +99,7 @@ const grepAt = (entry: Case, files: Map, pattern: string, scope: // Served files also exist in this checkout, so git applies its own pathspec rules to the scope. const matched = new Set((git(["ls-files", "--", scope || "."]) ?? "").split("\n")); const inScope = (path: string) => matched.has(path); - const fromRev = (git(["grep", "-n", "-E", pattern, entry.rev, "--", scope || "."]) ?? "") + const fromRev = (git(["grep", "-n", "-E", "-e", pattern, entry.rev, "--", scope || "."]) ?? "") .split("\n") .filter((line) => line !== "") .map((line) => line.replace(`${entry.rev}:`, ""))