From 1f2d720e1ee7fd6ff3607cc750933d68696c200e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 17:55:49 +0000 Subject: [PATCH 1/6] test: pin the rewrite formatter's indentation decisions Mutation testing scored 98.27 with six mutants standing, every one of them inside `formatImplementation`. They stood because no test ever promoted the shape a model actually returns: the existing cases use single-line bodies, or multi-line ones with no leading indent, so `Math.min` returned 0 whatever the de-dent logic did. Two of the six were equivalent -- `/^\s*/` always matches, so neither the `^` anchor nor the `?. ... ?? 0` guarding `exec` could ever be observed. That is dead defensive code, so it is gone, replaced by `leadingWhitespace`, which says what the expression was for. The new cases pin the rest: the de-dent is measured from the code lines and ignores the blank ones between them, a whitespace-only line counts as blank rather than as the shallowest indent, and a file that uses tabs supplies the indent unit for a method whose own indentation does not. Writing them surfaced a fault. Blank lines inside a promoted body were indented like any other, so promoting a multi-line candidate wrote trailing whitespace into the user's source file -- which their own lint step then rejects. Blank lines now emit empty. `digest` gets the same treatment: `isRecord`'s `typeof` guard is what keeps `undefined` away from `Object.getPrototypeOf`, which throws on it, and optional fields do reach the digest undefined -- candidate `metadata` is one. Nothing covered that. Mutation score is now 100 across the mutated scope, and the break threshold ratchets to match, with a note on excluding a genuinely equivalent mutant at the line rather than by lowering the threshold again. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XRoQAp6kcgCgMvb1jcY115 --- packages/rewrite/src/apply.ts | 23 ++++++++---- packages/rewrite/test/apply.test.ts | 49 +++++++++++++++++++++++-- packages/rewrite/test/canonical.test.ts | 10 +++++ stryker.config.json | 11 ++++-- 4 files changed, 79 insertions(+), 14 deletions(-) diff --git a/packages/rewrite/src/apply.ts b/packages/rewrite/src/apply.ts index 3e04cc8..01f9eb6 100644 --- a/packages/rewrite/src/apply.ts +++ b/packages/rewrite/src/apply.ts @@ -70,14 +70,23 @@ export function revertRewrite(source: string, snapshot: RewriteSnapshot): string } function formatImplementation(implementation: string, methodIndent: string, source: string): string { - // Match the method's own indentation style rather than the whole file's, - // so a tab-indented method in a mostly-spaces file still gets tabs. - const indentUnit = methodIndent.includes("\t") ? "\t" : source.includes("\t") ? "\t" : " "; + // The method's own indentation style wins, so a tab-indented method in a + // mostly-spaces file still gets tabs; the file is the fallback for a method + // whose own indent says nothing, as a top-level function's does not. + const indentUnit = methodIndent.includes("\t") || source.includes("\t") ? "\t" : " "; const bodyIndent = `${methodIndent}${indentUnit}`; const lines = implementation.split("\n"); - const minimumIndent = Math.min( - ...lines.filter((line) => line.trim()).map((line) => /^\s*/.exec(line)?.[0].length ?? 0), - ); - const normalized = lines.map((line) => `${bodyIndent}${line.slice(Number.isFinite(minimumIndent) ? minimumIndent : 0)}`); + // Blank lines carry no indentation to measure, and a blank line counted as + // zero would cancel the de-dent for every other line. `Math.min()` of + // nothing is Infinity, which is the all-blank body: de-dent by nothing. + const minimumIndent = Math.min(...lines.filter((line) => line.trim()).map(leadingWhitespace)); + // A blank line is emitted empty rather than indented: trailing whitespace in + // a file this library wrote is a lint failure in the user's own repository. + const normalized = lines.map((line) => + line.trim() ? `${bodyIndent}${line.slice(Number.isFinite(minimumIndent) ? minimumIndent : 0)}` : ""); return `\n${normalized.join("\n")}\n${methodIndent}`; } + +function leadingWhitespace(line: string): number { + return line.length - line.trimStart().length; +} diff --git a/packages/rewrite/test/apply.test.ts b/packages/rewrite/test/apply.test.ts index d1fa576..2c7e7ba 100644 --- a/packages/rewrite/test/apply.test.ts +++ b/packages/rewrite/test/apply.test.ts @@ -115,10 +115,12 @@ describe("body reindentation", () => { expect(result).not.toContain(' return "b";'); }); - it("preserves blank lines inside the body", () => { + it("preserves blank lines inside the body, and leaves them empty", () => { + // Indenting a blank line would write trailing whitespace into the user's + // source file, which their own lint step then rejects. const result = rewriteWith(spaceSource, 'const a = 1;\n\nreturn "b";'); - expect(result).toContain('\n const a = 1;\n'); - expect(result).toContain('\n return "b";\n }'); + expect(result).toContain('\n const a = 1;\n\n return "b";\n }'); + expect(result).not.toMatch(/[ \t]+\n/); }); it("handles a body that is only whitespace without collapsing the method", () => { @@ -127,6 +129,47 @@ describe("body reindentation", () => { expect(result.endsWith("}\n")).toBe(true); }); + // The mutants that survived the previous round all lived in the de-dent: + // every case above uses either a single-line body or a multi-line one with + // no leading indent, so `Math.min` returned 0 whatever the filter did. A + // model returns indented, blank-line-separated code, which is exactly the + // shape that tells the decisions apart. + it("measures the de-dent from the code lines, ignoring the blank ones between them", () => { + // Every code line is indented four; dropping the blank-line filter would + // measure zero and leave the whole body double-indented. + const result = rewriteWith(spaceSource, ' const a = 1;\n\n return "b";'); + expect(result).toContain('\n const a = 1;\n\n return "b";\n }'); + }); + + it("treats a whitespace-only line as blank, not as the shallowest indent", () => { + // The separator here is two spaces rather than empty. Filtering on the + // raw line instead of its trimmed form would measure it as an indent of + // two and de-dent every other line by two instead of four. + const result = rewriteWith(spaceSource, ' const a = 1;\n \n return "b";'); + expect(result).toContain('\n const a = 1;\n\n return "b";\n }'); + expect(result).not.toContain('\n const a = 1;'); + }); + + it("uses tabs when the file is tab-indented even where the method's own indent is not", () => { + // A method at column zero -- a top-level function, or the first method of + // a class written flush left -- has no indent of its own to copy, so the + // file decides. Nothing exercised this arm before: every tab fixture also + // had a tab-indented method. + const mixed = 'class T {\n\tother(): void {}\n}\nfunction m(): string {\n "use audit";\n return "a";\n}\n'; + const bodyStart = mixed.indexOf('"use audit";') + '"use audit";'.length; + const closing = "\n}"; + const target: RewriteTarget = { + id: "m", + artifactRef: "memory://mixed.ts", + bodyStart, + bodyEnd: mixed.lastIndexOf(closing) + closing.length - 1, + bodyDigest: digest(mixed.slice(bodyStart, mixed.lastIndexOf(closing) + closing.length - 1)), + indentation: "", + }; + const result = applyCandidate(mixed, { id: "mixed", target, implementation: 'return "b";' }); + expect(result).toContain('\n\treturn "b";\n}'); + }); + it("always opens with a newline and closes at the method's indent", () => { const result = rewriteWith(spaceSource, 'return "b";'); const bodyStart = result.indexOf('"use audit";') + '"use audit";'.length; diff --git a/packages/rewrite/test/canonical.test.ts b/packages/rewrite/test/canonical.test.ts index 5a5566d..b9385cc 100644 --- a/packages/rewrite/test/canonical.test.ts +++ b/packages/rewrite/test/canonical.test.ts @@ -40,6 +40,16 @@ describe("digest", () => { expect(digest(bare)).toBe(digest({ a: 2, b: 1 })); }); + it("hashes undefined rather than throwing on it", () => { + // `isRecord`'s `typeof` guard is what keeps `undefined` away from + // `Object.getPrototypeOf`, which throws on it. Optional fields reach the + // digest undefined -- candidate `metadata` is one -- so this is the + // ordinary case, not a hostile input. + expect(digest(undefined)).toMatch(/^sha256:[0-9a-f]{64}$/); + expect(digest({ metadata: undefined })).toBe(digest({})); + expect(digest([undefined])).toBe(digest([null])); + }); + it("does not canonicalize class instances into empty objects", () => { // A Date serializes through JSON.stringify; if isRecord wrongly accepted // it, every Date would hash identically. diff --git a/stryker.config.json b/stryker.config.json index 856037c..60fc2f9 100644 --- a/stryker.config.json +++ b/stryker.config.json @@ -27,9 +27,9 @@ "packages/rewrite/src/canonical.ts" ], "thresholds": { - "high": 98, - "low": 95, - "break": 95 + "high": 100, + "low": 100, + "break": 100 }, "_comment": [ "Scoped to the modules where a surviving mutant is alarming rather than merely untidy:", @@ -39,7 +39,10 @@ "the decision. Mutating everything would take hours and mostly re-measure line coverage,", "which vitest already enforces.", "The break threshold is a ratchet, like the coverage thresholds: raise it as suites", - "improve, never lower it to get a build green." + "improve, never lower it to get a build green. It reached 100 by killing the last", + "survivors in the rewrite formatter; a genuinely equivalent mutant -- one no test", + "could distinguish -- is excluded at the line with a `// Stryker disable next-line", + ": ` comment, which is reviewable, rather than by lowering this." ], "ignorePatterns": [ "test/output", From faa5ba600a2c5afb94a2f3a5937367f0f1002f71 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 18:06:51 +0000 Subject: [PATCH 2/6] test: cover the public API and guards nothing ever called MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A coverage sweep found whole decisions with no test behind them. Each of these is a public entry point or a guard that decides something consequential, not a line-count gap. **`defineGrounding`** — the runtime every emitted registration calls, and the only thing between generated source and a grounding naming no method, describing no intent, or pointing at no contract. `scan.test.ts` checked that codegen *emits* a call to it; nothing ever called it. All three validations, the freeze, and the ordering that keeps a blank methodRef out of the message that would interpolate it are now pinned. **`createSandboxPolicy`** — only `version` was ever asserted. The rest is the whole of the default confinement story: workspace-only writes, the outbound allowlist, the readonly paths, the UI lockdown. The allowlist matters most and now has the case that would be worst to get wrong — an allowlist that filters down to nothing fails closed rather than reading as "no restrictions". The policy also copies the caller's arrays rather than aliasing them, which nothing checked. **`CandidateEngine`'s request validation** — the guards keeping one trainable's records and evaluations out of another's optimization. A request assembled with the wrong evidence trains a method on traffic it never served; both guards were unexecuted. **`rewritePromotion`'s hot swap** — the applier writes the source rewrite *and* installs a live implementation so a running process picks the candidate up without a restart. The second half had no test at all, though it is the half that changes an application already serving traffic. Covered now: the swap installs for an async target, forwards arguments and the receiver to the executor, comes back out on rollback, and never installs for a sync target (whose calling convention it would change) or without an executor. **`wrapTrainable`'s wrapper** — the load-time half of the zero-config flow, for a `"use training"` function rather than a method. Every test of that flow installs a stub `wrap` handler, so the real wrapper was built and never called. **The Ax engine's examples and metric** — the branch turning captured traffic into examples is the zero-config path the README leads with, and it never ran; neither did the metric's failure modes. A metric that scores everything 1 optimizes nothing, so an empty body, a body that throws, and a wrong answer now each have a case. **The CLI** — `status`'s counting loop only ever ran against zero records, and a discovery failure the library does not model was never shown to propagate rather than turning into a tidy exit code. Two faults surfaced while writing these: - `inputValue` substring-matched the declared parameter type independently of `fieldType`, so the two disagreed wherever a type merely contains a primitive name. `Record` and `string[]` were declared to Ax as a `json` field and a string array, then handed a JSON string. It now derives from `fieldType`, so the value matches the field. - `test/promotion-applier.test.ts` needed explicit teardown: the swap registry is module-global and keyed by trainable id, so "does not swap" assertions would otherwise pass or fail on declaration order. Coverage: statements 93.22 -> 96.15, branches 83.29 -> 88.60, functions 95.48 -> 97.18, lines 95.88 -> 98.20. 726 -> 803 tests. Mutation score holds at 100. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XRoQAp6kcgCgMvb1jcY115 --- packages/grounding/test/decorators.test.ts | 68 ++++ packages/harness/test/harness.test.ts | 74 +++- packages/rewrite/test/weaving.test.ts | 37 ++ packages/training/test/engine.test.ts | 77 ++++ src/providers/ax.ts | 18 +- test/ax.test.ts | 410 ++++++++++++++++++++- test/cli.test.ts | 51 ++- test/instrumentation.test.ts | 60 +++ test/promotion-applier.test.ts | 181 +++++++++ 9 files changed, 970 insertions(+), 6 deletions(-) create mode 100644 test/promotion-applier.test.ts diff --git a/packages/grounding/test/decorators.test.ts b/packages/grounding/test/decorators.test.ts index 3089034..e38ace6 100644 --- a/packages/grounding/test/decorators.test.ts +++ b/packages/grounding/test/decorators.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { composeOptions, + defineGrounding, finalizeTrainableClass, granularOptionsFor, intent, @@ -102,3 +103,70 @@ describe("granular grounding decorators", () => { expect(refs.sort()).toEqual(["Bare.one", "Bare.two"]); }); }); + +describe("defineGrounding", () => { + // The runtime entry point every emitted registration calls, and the only + // thing standing between generated source and a grounding that names no + // method, describes no intent, or points at no contract. Nothing exercised + // its validation: `scan.test.ts` checks that codegen *emits* a + // `defineGrounding(...)` call, never that calling it rejects anything. + + const valid: GroundingOptions = { + methodRef: "Router.route", + intent: "Route the input", + contract: { ref: "decl://Router.route" }, + }; + + it("returns the options it was given for a complete grounding", () => { + expect(defineGrounding(valid)).toEqual(valid); + }); + + it("freezes the result and its contract, so a registry cannot be edited through it", () => { + const grounding = defineGrounding({ ...valid, params: { input: param("The input") } }); + expect(Object.isFrozen(grounding)).toBe(true); + expect(Object.isFrozen(grounding.contract)).toBe(true); + }); + + it("keeps the optional params and output it was given", () => { + const grounding = defineGrounding({ + ...valid, + params: { input: param("The input") }, + output: { returns: { description: "The route" } }, + }); + expect(grounding.params).toEqual({ input: { description: "The input" } }); + expect(grounding.output).toEqual({ returns: { description: "The route" } }); + }); + + it.each([ + ["undefined", undefined], + ["empty", ""], + ["only whitespace", " "], + ])("rejects a methodRef that is %s", (_label, methodRef) => { + expect(() => defineGrounding({ ...valid, methodRef: methodRef as string })) + .toThrow(new TypeError("grounding methodRef must be a non-empty string")); + }); + + it.each([ + ["undefined", undefined], + ["empty", ""], + ["only whitespace", "\t "], + ])("rejects an intent that is %s, naming the method", (_label, intentText) => { + expect(() => defineGrounding({ ...valid, intent: intentText as string })) + .toThrow(/grounding intent must be a non-empty string for Router\.route/); + }); + + it.each([ + ["absent", undefined], + ["blank", { ref: " " }], + ["an empty ref", { ref: "" }], + ])("rejects a contract that is %s, naming the method", (_label, contract) => { + expect(() => defineGrounding({ ...valid, contract: contract as GroundingOptions["contract"] })) + .toThrow(/grounding contract ref must be a non-empty string for Router\.route/); + }); + + it("checks the methodRef before the message that would interpolate it", () => { + // A grounding missing everything must not report a blank method name. + expect(() => defineGrounding({} as GroundingOptions)) + .toThrow("grounding methodRef must be a non-empty string"); + }); +}); diff --git a/packages/harness/test/harness.test.ts b/packages/harness/test/harness.test.ts index ec3bccd..8f9223a 100644 --- a/packages/harness/test/harness.test.ts +++ b/packages/harness/test/harness.test.ts @@ -1,7 +1,7 @@ import { mkdtemp, readFile, stat, symlink, writeFile } from "node:fs/promises"; import { createRequire } from "node:module"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { join, resolve } from "node:path"; import { describe, expect, it, vi } from "vitest"; @@ -274,6 +274,78 @@ describe("training harness", () => { expect(createSandboxPolicy({ workspace: tmpdir(), version: "0.6.0-alpha" }).version).toBe("0.6.0-alpha"); }); + // The policy builder is the whole of the default confinement story, and only + // its `version` field was ever asserted. Every other branch -- the outbound + // allowlist above all -- decides what a model-driven agent can reach. + describe("the default policy it builds", () => { + it("confines writes to the workspace and denies the network outright", () => { + const policy = createSandboxPolicy({ workspace: tmpdir() }); + expect(policy.filesystem).toEqual({ readwritePaths: [tmpdir()] }); + expect(policy.network).toEqual({ allowOutbound: false, allowLocalNetwork: false }); + expect(policy.ui).toEqual({ allowWindows: false, clipboard: "none", allowInputInjection: false }); + expect(policy.timeoutMs).toBeUndefined(); + }); + + it("adds readonly paths only when some were given", () => { + const readonlyPaths = [tmpdir(), resolve(tmpdir(), "vendor")]; + expect(createSandboxPolicy({ workspace: tmpdir(), readonlyPaths }).filesystem?.readonlyPaths) + .toEqual(readonlyPaths); + expect(createSandboxPolicy({ workspace: tmpdir(), readonlyPaths: [] }).filesystem) + .not.toHaveProperty("readonlyPaths"); + }); + + it("copies the readonly paths rather than aliasing the caller's array", () => { + const readonlyPaths = [resolve(tmpdir(), "vendor")]; + const policy = createSandboxPolicy({ workspace: tmpdir(), readonlyPaths }); + readonlyPaths.push(resolve(tmpdir(), "smuggled")); + expect(policy.filesystem?.readonlyPaths).toEqual([resolve(tmpdir(), "vendor")]); + }); + + it("opens outbound access only for the hosts it was given, never the local network", () => { + const policy = createSandboxPolicy({ workspace: tmpdir(), allowedHosts: ["api.openai.com"] }); + expect(policy.network).toEqual({ + allowOutbound: true, + allowLocalNetwork: false, + allowedHosts: ["api.openai.com"], + }); + }); + + it("copies the allowed hosts rather than aliasing the caller's array", () => { + const allowedHosts = ["api.openai.com"]; + const policy = createSandboxPolicy({ workspace: tmpdir(), allowedHosts }); + allowedHosts.push("evil.example"); + expect(policy.network).toMatchObject({ allowedHosts: ["api.openai.com"] }); + }); + + it("keeps the network closed when the allowlist is empty or only blanks", () => { + // An allowlist that filters down to nothing must fail closed. Reading + // it as "no restrictions" would open the network on a typo. + for (const allowedHosts of [[], ["", " "]]) { + expect(createSandboxPolicy({ workspace: tmpdir(), allowedHosts }).network) + .toEqual({ allowOutbound: false, allowLocalNetwork: false }); + } + }); + + it("drops blank entries from an otherwise real allowlist", () => { + expect(createSandboxPolicy({ workspace: tmpdir(), allowedHosts: ["api.openai.com", " "] }).network) + .toMatchObject({ allowedHosts: ["api.openai.com"] }); + }); + + it("carries a timeout only when one was configured", () => { + expect(createSandboxPolicy({ workspace: tmpdir(), timeoutMs: 30_000 }).timeoutMs).toBe(30_000); + expect(createSandboxPolicy({ workspace: tmpdir() })).not.toHaveProperty("timeoutMs"); + }); + + it.each([ + ["a relative workspace", { workspace: "relative/path" }], + ["a relative readonly path", { workspace: tmpdir(), readonlyPaths: ["relative/path"] }], + ["a zero timeout", { workspace: tmpdir(), timeoutMs: 0 }], + ["a fractional timeout", { workspace: tmpdir(), timeoutMs: 1.5 }], + ])("refuses %s rather than building a policy around it", (_label, settings) => { + expect(() => createSandboxPolicy(settings as Parameters[0])).toThrow(); + }); + }); + it("refuses symlinked paths that resolve outside the workspace", async () => { const workspace = await mkdtemp(join(tmpdir(), "ts-autocode-sandbox-links-")); const outside = await mkdtemp(join(tmpdir(), "ts-autocode-outside-")); diff --git a/packages/rewrite/test/weaving.test.ts b/packages/rewrite/test/weaving.test.ts index 9987de4..ede63df 100644 --- a/packages/rewrite/test/weaving.test.ts +++ b/packages/rewrite/test/weaving.test.ts @@ -24,6 +24,7 @@ describe("rewrite weaving", () => { restoreImplementation("Router.fallback"); restoreImplementation("Static.echo"); restoreImplementation("free.normalize"); + restoreImplementation("free.malformed"); }); it("weaves annotated methods and leaves sibling methods untouched", () => { @@ -130,9 +131,45 @@ describe("rewrite weaving", () => { swapImplementation("free.normalize", (input) => String(input).trim().toUpperCase()); expect(call(" x ")).toBe("X"); restoreImplementation("free.normalize"); + restoreImplementation("free.malformed"); expect(call(" x ")).toBe("x"); }); + it("ignores a method the class does not declare, rather than throwing at load time", () => { + // Weaving is driven by parsed source and by decorators, so a name that + // resolves to nothing means the caller's discovery disagreed with the + // class. Throwing here would take down module load for the whole + // application; the method simply is not woven. + class Router { + route(input: string): string { return input; } + } + expect(() => annotateRewrite(Router, "missing", "Router.missing", MARKER)).not.toThrow(); + + const seen: string[] = []; + configureRewrite({ marker: MARKER, intercept: (invocation) => { seen.push(invocation.id); return invocation.proceed(); } }); + expect(new Router().route("abc")).toBe("abc"); + expect(seen).toEqual([]); + }); + + it("dispatches under a marker that is not a well-formed directive without throwing", () => { + // `normalizeMarker` rejects anything that is not `"use "`. Dispatch + // must not inherit that rejection: a malformed marker has no registered + // configuration, so the call runs its original implementation. + const original = vi.fn((input: string) => `original:${input}`); + expect(dispatchRewrite("free.malformed", "not a directive", "normalize", original as never, undefined, ["abc"])) + .toBe("original:abc"); + expect(original).toHaveBeenCalledOnce(); + + // A swap still applies: the swap registry is keyed by id, not by marker. + swapImplementation("free.malformed", (input) => `swapped:${String(input)}`); + try { + expect(dispatchRewrite("free.malformed", "not a directive", "normalize", original as never, undefined, ["abc"])) + .toBe("swapped:abc"); + } finally { + restoreImplementation("free.malformed"); + } + }); + it("routes each marker to its own configured interceptor", () => { class Router { route(input: string): string { return input; } diff --git a/packages/training/test/engine.test.ts b/packages/training/test/engine.test.ts index 4475ad2..16c9b79 100644 --- a/packages/training/test/engine.test.ts +++ b/packages/training/test/engine.test.ts @@ -6,6 +6,7 @@ import { defineTrainable, type CandidatePatch, type TrainingEngine, + type TrainingRecord, } from "../src/index.js"; import { CandidateEngine } from "../src/engine.js"; import { discoverInSource } from "../src/source.js"; @@ -56,6 +57,82 @@ describe("provider-neutral engine", () => { expect(() => applyCandidate(changed, candidate)).toThrow("changed after discovery"); }); + // `#validateRequest` is what keeps one trainable's evidence out of another's + // optimization. A request assembled with the wrong records trains a method + // on traffic it never served, and the resulting candidate is scored against + // the wrong behavior -- so these guards fail the request rather than + // proposing from it. Only the objective guard was covered. + describe("request validation", () => { + const engine: TrainingEngine = { id: "guard", async optimize() { return { implementation: "return input;" }; } }; + const other = defineTrainable("Other.route"); + + function request(overrides: Partial[0]> = {}) { + return { + trainableId: token.id, + objective: "uppercase", + target, + records: [], + evaluations: [], + ...overrides, + }; + } + + function record(trainableId: TrainingRecord["trainableId"]): TrainingRecord { + return { + id: "r1", + runId: "run-1", + trainableId, + method: "route", + succeeded: true, + recordedAt: new Date(0).toISOString(), + trace: { messages: [] } as unknown as TrainingRecord["trace"], + }; + } + + it.each([ + ["empty", ""], + ["only whitespace", " \t "], + ])("refuses an objective that is %s", async (_label, objective) => { + await expect(new CandidateEngine(engine).propose(request({ objective }), { variables: {} })) + .rejects.toThrow("optimization objective must be a non-empty string"); + }); + + it("refuses a target that is not the trainable the request names", async () => { + await expect(new CandidateEngine(engine).propose( + request({ trainableId: other.id }), + { variables: {} }, + )).rejects.toThrow("trainable target must match the request id"); + }); + + it("refuses records captured from a different trainable", async () => { + await expect(new CandidateEngine(engine).propose( + request({ records: [record(token.id), record(other.id)] }), + { variables: {} }, + )).rejects.toThrow("training records must match the request id"); + }); + + it("refuses evaluations bound to a different trainable", async () => { + await expect(new CandidateEngine(engine).propose( + request({ evaluations: [{ trainableId: other.id, result: {} as never }] }), + { variables: {} }, + )).rejects.toThrow("evaluations must match the request id"); + }); + + it("accepts records and evaluations that all name the request's trainable", async () => { + await expect(new CandidateEngine(engine).propose( + request({ + records: [record(token.id)], + evaluations: [{ trainableId: token.id, result: {} as never }], + }), + { variables: {} }, + )).resolves.toMatchObject({ trainableId: token.id }); + }); + + it("refuses an engine whose own id is blank, before it is ever asked", () => { + expect(() => new CandidateEngine({ ...engine, id: " " })).toThrow(); + }); + }); + it("rejects invalid TypeScript returned by an engine", async () => { const engine: TrainingEngine = { id: "invalid", diff --git a/src/providers/ax.ts b/src/providers/ax.ts index a3177a8..09482c0 100644 --- a/src/providers/ax.ts +++ b/src/providers/ax.ts @@ -302,10 +302,22 @@ function contentText(content: unknown): string | undefined { return content === undefined ? undefined : JSON.stringify(content); } +/** Shapes one captured argument as the Ax field declared for its parameter. + * + * This derives from {@link fieldType} rather than substring-matching the + * declared type a second time. Matching independently made the two disagree + * wherever a type merely *contains* a primitive name: `Record` + * and `string[]` were declared as a `json` field and a string array, then + * handed a JSON string, because both contain "string". Ax then received a value + * of a different shape than the signature it was given. */ function inputValue(value: unknown, type: string): unknown { - if (type.includes("string")) return typeof value === "string" ? value : JSON.stringify(value); - if (type.includes("number")) return Number(value); - if (type.includes("boolean")) return Boolean(value); + const field = fieldType(type); + // A captured argument that is not an array where one is declared is wrapped + // rather than dropped, matching how `argsFromContent` reads a bare JSON value. + if (field.isArray) return Array.isArray(value) ? value : value === undefined ? [] : [value]; + if (field.name === "string") return typeof value === "string" ? value : JSON.stringify(value); + if (field.name === "number") return Number(value); + if (field.name === "boolean") return Boolean(value); return value ?? null; } diff --git a/test/ax.test.ts b/test/ax.test.ts index 8b1e932..af6d5a3 100644 --- a/test/ax.test.ts +++ b/test/ax.test.ts @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { defineTrainable, type BoundEvaluation } from "../src/index.js"; +import { defineTrainable, type BoundEvaluation, type TrainingRecord } from "../src/index.js"; import { apiKeyNamesFor, createAxEngine } from "../src/providers/ax.js"; import { discoverInSource } from "ts-autocode-training"; @@ -203,3 +203,411 @@ describe("model selection", () => { })).rejects.toThrow("model.service must be an AxAIService"); }); }); + +// ---------------------------------------------------------------- examples +// +// What the engine hands Ax to optimize against is the whole substance of the +// default engine: everything else is provider plumbing. The suites above only +// ever pass one AgentV evaluation, so the branch that turns *captured traffic* +// into examples -- the zero-config path the README leads with -- was never +// executed, and neither was any of the content decoding underneath it. + +function trace(messages: ReadonlyArray<{ role: string; content: unknown }>): TrainingRecord["trace"] { + return { messages } as unknown as TrainingRecord["trace"]; +} + +function captured(overrides: Partial = {}): TrainingRecord { + return { + id: "record-1", + runId: "run-1", + trainableId: token.id, + method: "route", + succeeded: true, + recordedAt: new Date(0).toISOString(), + trace: trace([ + { role: "user", content: '["hello"]' }, + { role: "assistant", content: "HELLO" }, + ]), + ...overrides, + }; +} + +/** The examples the engine handed the optimizer for a given request. */ +async function examplesFor(request: Parameters["optimize"]>[0]) { + const engine = createAxEngine({ studentAI: {} as never }); + await engine.optimize(request, { variables: {} }).catch(() => undefined); + return (mocks.optimize.mock.calls[0]?.[1] ?? []) as Array>; +} + +describe("examples the engine optimizes against", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.ax.mockReturnValue({ applyOptimization: mocks.applyOptimization, forward: mocks.forward }); + mocks.forward.mockResolvedValue({ optimizedMethodImplementation: "return input.toUpperCase();" }); + mocks.optimize.mockResolvedValue({ optimizedProgram: {} }); + }); + + it("turns a successful captured call into an example of its arguments and result", async () => { + const examples = await examplesFor({ + trainableId: token.id, objective: "improve", target, records: [captured()], evaluations: [], + }); + expect(examples).toHaveLength(1); + expect(examples[0]).toMatchObject({ + methodArgumentInput: "hello", + trainingArgumentsJson: '["hello"]', + expectedMethodOutput: "HELLO", + trainingObjective: "improve", + }); + }); + + it("skips a captured call that failed, which demonstrates nothing to reproduce", async () => { + await expect(examplesFor({ + trainableId: token.id, + objective: "improve", + target, + records: [captured({ succeeded: false })], + evaluations: [], + })).resolves.toEqual([]); + }); + + it("skips a capture with no assistant turn to learn an expected output from", async () => { + await expect(examplesFor({ + trainableId: token.id, + objective: "improve", + target, + records: [captured({ trace: trace([{ role: "user", content: '["hello"]' }]) })], + evaluations: [], + })).resolves.toEqual([]); + }); + + it("learns from the last assistant turn, not the first", async () => { + const examples = await examplesFor({ + trainableId: token.id, + objective: "improve", + target, + records: [captured({ + trace: trace([ + { role: "user", content: '["hello"]' }, + { role: "assistant", content: "stale" }, + { role: "assistant", content: "HELLO" }, + ]), + })], + evaluations: [], + }); + expect(examples[0]).toMatchObject({ expectedMethodOutput: "HELLO" }); + }); + + it("de-duplicates captures of the same arguments, however often they were served", async () => { + const examples = await examplesFor({ + trainableId: token.id, + objective: "improve", + target, + records: [captured(), captured({ id: "record-2" }), captured({ id: "record-3" })], + evaluations: [], + }); + expect(examples).toHaveLength(1); + }); + + it("refuses to optimize with nothing to learn from, naming the trainable", async () => { + const engine = createAxEngine({ studentAI: {} as never }); + await expect(engine.optimize( + { trainableId: token.id, objective: "improve", target, records: [], evaluations: [] }, + { variables: {} }, + )).rejects.toThrow(token.id); + expect(mocks.optimize).not.toHaveBeenCalled(); + }); + + describe("decoding what a trace carries as content", () => { + it.each([ + ["a JSON array, as the arguments themselves", '["hello"]', "hello"], + ["a bare JSON value, as a single argument", '"hello"', "hello"], + ["text that is not JSON, as one string argument", "hello", "hello"], + ])("reads %s", async (_label, content, expected) => { + const examples = await examplesFor({ + trainableId: token.id, + objective: "improve", + target, + records: [captured({ trace: trace([{ role: "user", content }, { role: "assistant", content: "HELLO" }]) })], + evaluations: [], + }); + expect(examples[0]).toMatchObject({ methodArgumentInput: expected }); + }); + + it("joins the text parts of a multi-part content block", async () => { + const examples = await examplesFor({ + trainableId: token.id, + objective: "improve", + target, + records: [captured({ + trace: trace([ + { role: "user", content: ['["hel', { text: 'lo"]' }] }, + { role: "assistant", content: [{ text: "HEL" }, "LO"] }, + ]), + })], + evaluations: [], + }); + expect(examples[0]).toMatchObject({ methodArgumentInput: "hello", expectedMethodOutput: "HELLO" }); + }); + + it("serializes a structured content block rather than dropping it", async () => { + const examples = await examplesFor({ + trainableId: token.id, + objective: "improve", + target, + records: [captured({ + trace: trace([ + { role: "user", content: { note: "not text" } }, + { role: "assistant", content: "HELLO" }, + ]), + })], + evaluations: [], + }); + expect(examples[0]).toMatchObject({ methodArgumentInput: '{"note":"not text"}' }); + }); + }); + + describe("falling back to an evaluation's own result", () => { + it("uses the recorded output when a test carries no `equals` assertion", async () => { + const examples = await examplesFor({ + trainableId: token.id, + objective: "improve", + target, + records: [], + evaluations: [{ + trainableId: token.id, + test: { id: "t", input: '["hello"]', assert: [{ type: "contains", value: "H" }] }, + result: { input: [{ role: "user", content: '["hello"]' }], output: "HELLO", executionStatus: "ok" }, + } as unknown as BoundEvaluation], + }); + expect(examples[0]).toMatchObject({ expectedMethodOutput: "HELLO" }); + }); + + it("skips an evaluation whose run did not execute cleanly and asserted nothing", async () => { + await expect(examplesFor({ + trainableId: token.id, + objective: "improve", + target, + records: [], + evaluations: [{ + trainableId: token.id, + test: { id: "t", input: '["hello"]', assert: [] }, + result: { input: [{ role: "user", content: '["hello"]' }], output: "HELLO", executionStatus: "error" }, + } as unknown as BoundEvaluation], + })).resolves.toEqual([]); + }); + + it("reads the arguments from the run's own messages when the test has no input", async () => { + const examples = await examplesFor({ + trainableId: token.id, + objective: "improve", + target, + records: [], + evaluations: [{ + trainableId: token.id, + result: { input: [{ role: "user", content: '["hello"]' }], output: "HELLO", executionStatus: "ok" }, + } as unknown as BoundEvaluation], + }); + expect(examples[0]).toMatchObject({ methodArgumentInput: "hello" }); + }); + + it("falls back to the first message when no turn is marked as the user's", async () => { + const examples = await examplesFor({ + trainableId: token.id, + objective: "improve", + target, + records: [], + evaluations: [{ + trainableId: token.id, + result: { input: [{ role: "system", content: '["hello"]' }], output: "HELLO", executionStatus: "ok" }, + } as unknown as BoundEvaluation], + }); + expect(examples[0]).toMatchObject({ methodArgumentInput: "hello" }); + }); + }); +}); + +// ------------------------------------------------------------------ metric +// +// The metric is what the optimizer actually optimizes: it runs each candidate +// body and scores it against what the captured or evaluated call produced. It +// was only ever asserted through one inline expectation on the happy path, so +// none of the ways a candidate fails to earn a point were checked -- and a +// metric that scores everything 1 optimizes nothing. + +/** The scoring function the engine handed the optimizer for this request. */ +async function metricFor(records: TrainingRecord[]): Promise< + (input: { prediction: unknown; example: unknown }) => Promise +> { + let captured: ((input: { prediction: unknown; example: unknown }) => Promise) | undefined; + let example: unknown; + mocks.optimize.mockImplementation(async (_program, examples: unknown[], metric) => { + captured = metric as typeof captured; + example = examples[0]; + return { optimizedProgram: {} }; + }); + const engine = createAxEngine({ studentAI: {} as never }); + await engine.optimize( + { trainableId: token.id, objective: "uppercase", target, records, evaluations: [] }, + { variables: {} }, + ).catch(() => undefined); + if (!captured) throw new Error("the engine never handed the optimizer a metric"); + const metric = captured; + return (input) => metric({ ...input, example: input.example ?? example }); +} + +describe("the metric the engine hands the optimizer", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.ax.mockReturnValue({ applyOptimization: mocks.applyOptimization, forward: mocks.forward }); + mocks.forward.mockResolvedValue({ optimizedMethodImplementation: "return input;" }); + }); + + it("scores a candidate that reproduces the captured output", async () => { + const score = await metricFor([captured()]); + await expect(score({ + prediction: { optimizedMethodImplementation: "return input.toUpperCase();" }, + example: undefined, + })).resolves.toBe(1); + }, 30_000); + + it("scores a candidate whose output differs at zero", async () => { + const score = await metricFor([captured()]); + await expect(score({ + prediction: { optimizedMethodImplementation: "return input;" }, + example: undefined, + })).resolves.toBe(0); + }, 30_000); + + it.each([ + ["an empty body", ""], + ["a body of only whitespace", " \n\t"], + ])("scores %s at zero without running anything", async (_label, implementation) => { + const score = await metricFor([captured()]); + await expect(score({ + prediction: { optimizedMethodImplementation: implementation }, + example: undefined, + })).resolves.toBe(0); + }, 30_000); + + it("scores a missing prediction at zero rather than throwing inside the optimizer", async () => { + const score = await metricFor([captured()]); + await expect(score({ prediction: undefined, example: undefined })).resolves.toBe(0); + }, 30_000); + + it("scores a candidate that throws at zero, leaving the round to continue", async () => { + // A model returns code that does not run more often than code that runs + // and is wrong. Letting that reject would abort the whole optimization. + const score = await metricFor([captured()]); + await expect(score({ + prediction: { optimizedMethodImplementation: 'throw new Error("boom");' }, + example: undefined, + })).resolves.toBe(0); + }, 30_000); +}); + +describe("typing example values from the method signature", () => { + // `inputValue` coerces each captured argument to the type its parameter + // declares, so Ax receives a number field as a number rather than as the + // string the JSON trace round-tripped it through. Only the string arm ran. + const typedSource = `class Router { + page(index: number, deep: boolean, extra: Record, tags: string[]): string { + "use training"; + return String(index); + } +}`; + const typedTarget = discoverInSource(typedSource, "src/typed.ts")[0]!; + const typedToken = defineTrainable("Router.page"); + + beforeEach(() => { + vi.clearAllMocks(); + mocks.ax.mockReturnValue({ applyOptimization: mocks.applyOptimization, forward: mocks.forward }); + mocks.forward.mockResolvedValue({ optimizedMethodImplementation: "return String(index);" }); + mocks.optimize.mockResolvedValue({ optimizedProgram: {} }); + }); + + it("coerces each argument to the type its parameter declares", async () => { + const engine = createAxEngine({ studentAI: {} as never }); + await engine.optimize({ + trainableId: typedToken.id, + objective: "improve", + target: typedTarget, + records: [captured({ + trainableId: typedToken.id, + trace: trace([ + { role: "user", content: '["7", "yes", {"a":1}, ["x","y"]]' }, + { role: "assistant", content: "7" }, + ]), + })], + evaluations: [], + }, { variables: {} }).catch(() => undefined); + + const examples = (mocks.optimize.mock.calls[0]?.[1] ?? []) as Array>; + expect(examples[0]).toMatchObject({ + methodArgumentIndex: 7, + methodArgumentDeep: true, + // Declared `json` and `string[]`, so they arrive as an object and an + // array -- not as the JSON strings a second substring match produced. + methodArgumentExtra: { a: 1 }, + methodArgumentTags: ["x", "y"], + }); + }); + + it("wraps a captured argument that is not an array where the signature declares one", async () => { + const engine = createAxEngine({ studentAI: {} as never }); + await engine.optimize({ + trainableId: typedToken.id, + objective: "improve", + target: typedTarget, + records: [captured({ + trainableId: typedToken.id, + trace: trace([ + { role: "user", content: '[1, true, {}, "solo"]' }, + { role: "assistant", content: "1" }, + ]), + })], + evaluations: [], + }, { variables: {} }).catch(() => undefined); + + const examples = (mocks.optimize.mock.calls[0]?.[1] ?? []) as Array>; + expect(examples[0]).toMatchObject({ methodArgumentTags: ["solo"] }); + }); + + it("passes a missing argument as an empty array where one is declared", async () => { + const engine = createAxEngine({ studentAI: {} as never }); + await engine.optimize({ + trainableId: typedToken.id, + objective: "improve", + target: typedTarget, + records: [captured({ + trainableId: typedToken.id, + trace: trace([{ role: "user", content: "[1]" }, { role: "assistant", content: "1" }]), + })], + evaluations: [], + }, { variables: {} }).catch(() => undefined); + + const examples = (mocks.optimize.mock.calls[0]?.[1] ?? []) as Array>; + expect(examples[0]).toMatchObject({ methodArgumentTags: [], methodArgumentExtra: null }); + }); + + it("declares the Ax field type each parameter maps to", async () => { + const engine = createAxEngine({ studentAI: {} as never }); + await engine.optimize({ + trainableId: typedToken.id, + objective: "improve", + target: typedTarget, + records: [captured({ trainableId: typedToken.id })], + evaluations: [], + }, { variables: {} }).catch(() => undefined); + + const signature = mocks.ax.mock.calls[0]?.[0] as { + inputs: Array<{ name: string; type?: { name: string; isArray?: boolean } }>; + }; + expect(signature.inputs.slice(0, 4).map((field) => [field.name, field.type?.name, field.type?.isArray ?? false])) + .toEqual([ + ["methodArgumentIndex", "number", false], + ["methodArgumentDeep", "boolean", false], + ["methodArgumentExtra", "json", false], + ["methodArgumentTags", "string", true], + ]); + }); +}); diff --git a/test/cli.test.ts b/test/cli.test.ts index 3b1a351..e986ee9 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -1,11 +1,29 @@ import { mkdir, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { describeTrainables, run, usage } from "../src/cli.js"; import { discoverInSource } from "ts-autocode-training"; +// Discovery is the CLI's only failure surface, and the one behavior nothing +// covered is what happens when it fails in a way the library does not model. +// The rule the code states -- library errors are printed, anything else is a +// real crash and must not be swallowed into a tidy exit code -- needs a +// discovery that throws something else, which only a stub can produce. +const discovery = vi.hoisted(() => ({ crash: undefined as Error | undefined })); + +vi.mock("ts-autocode-training", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + discoverTrainables: (settings: Parameters[0]) => { + if (discovery.crash) throw discovery.crash; + return actual.discoverTrainables(settings); + }, + }; +}); + // Inspecting what is trainable required writing a script that imports // discoverTrainables. That matters more here than in most libraries, because // the identity a user must pass to train() is an exact string with no type @@ -85,6 +103,25 @@ describe("ts-autocode status", () => { expect(result.stdout).toContain("0 successful / 0 captured"); }); + it("renders the counts as a table when --json is not asked for", async () => { + // The table path only ever ran with no records at all, so the counting + // loop behind it -- the whole of what `status` reports -- was unexecuted. + const artifacts = join(directory, "table-artifacts"); + await rm(artifacts, { recursive: true, force: true }); + await mkdir(artifacts, { recursive: true }); + await writeFile(join(artifacts, "records.json"), JSON.stringify([ + { trainableId: "Router.route", succeeded: true }, + { trainableId: "Router.route", succeeded: true }, + { trainableId: "Router.route", succeeded: false }, + ]), "utf8"); + const result = await run(["status", "--file", await project(), "--output-dir", artifacts]); + + expect(result.code).toBe(0); + expect(result.stdout).toContain("Router.route 2 successful / 3 captured"); + // A trainable with no records still appears, at zero. + expect(result.stdout).toContain("Router.enrich 0 successful / 0 captured"); + }); + it("counts captured traces per trainable", async () => { const artifacts = join(directory, "artifacts"); await rm(artifacts, { recursive: true, force: true }); @@ -124,6 +161,18 @@ describe("ts-autocode argument handling", () => { expect(result.code).toBe(1); expect(result.stderr).not.toContain(" at "); }); + + describe("a failure the library does not model", () => { + afterEach(() => { discovery.crash = undefined; }); + + it.each(["discover", "status"])("propagates out of %s rather than becoming an exit code", async (command) => { + // Swallowing this would report "nothing found" for a broken install, + // which is the least debuggable outcome available. + discovery.crash = new TypeError("something the library does not model"); + await expect(run([command, "--file", await project()])) + .rejects.toThrow("something the library does not model"); + }); + }); }); describe("ts-autocode status output paths", () => { diff --git a/test/instrumentation.test.ts b/test/instrumentation.test.ts index 0b0950f..9d5cc26 100644 --- a/test/instrumentation.test.ts +++ b/test/instrumentation.test.ts @@ -4,8 +4,12 @@ import { configureTraining, defineTrainable, instrumentTrainable, + restoreImplementation, + swapImplementation, + toTrainableToken, trainable, training, + wrapTrainable, } from "../src/index.js"; describe("instrumentation wiring", () => { @@ -42,6 +46,62 @@ describe("instrumentation wiring", () => { }); }); +describe("wrapping a directive-marked free function", () => { + // `wrapTrainable` is the load-time half of the zero-config flow: what + // `ts-autocode/register` calls for a `"use training"` function rather than a + // class method. Every test of that flow installs a stub `wrap` handler, so + // the real wrapper was built but never called -- the capture, the identity + // stamp and the hot-swap it exists to route through were all unexercised. + + it("returns what the function returns, and captures the call", async () => { + configureTraining({ tracing: { enabled: false } }); + const wrapped = wrapTrainable((input: string) => input.toUpperCase(), "Free.shout"); + + expect(wrapped("billing")).toBe("BILLING"); + const records = await training.records(defineTrainable("Free.shout")); + expect(records).toHaveLength(1); + expect(records[0]).toMatchObject({ trainableId: "Free.shout", succeeded: true }); + }); + + it("lets a failure through unchanged, and records it as a failure", async () => { + configureTraining({ tracing: { enabled: false } }); + const wrapped = wrapTrainable(() => { throw new Error("boom"); }, "Free.fails"); + + expect(() => wrapped()).toThrow("boom"); + expect(await training.records(defineTrainable("Free.fails"))).toMatchObject([{ succeeded: false }]); + }); + + it("is idempotent: wrapping an already-wrapped function hands back the same one", () => { + const wrapped = wrapTrainable((input: string) => input, "Free.once"); + expect(wrapTrainable(wrapped, "Free.once")).toBe(wrapped); + }); + + it("keeps the function's own name, falling back to the id for an anonymous one", () => { + function named(input: string): string { return input; } + expect(wrapTrainable(named, "Free.named").name).toBe("named"); + expect(wrapTrainable(((input: string) => input) as { (input: string): string; name?: string }, "Free.arrow").name) + .toBe("Free.arrow"); + }); + + it("stamps the wrapper with its identity, so train() resolves it without a retyped string", () => { + const wrapped = wrapTrainable((input: string) => input, "Free.stamped"); + expect(toTrainableToken(wrapped).id).toBe("Free.stamped"); + }); + + it("routes through a hot-swapped implementation once one is promoted", () => { + const wrapped = wrapTrainable((input: string) => input, "Free.swappable"); + expect(wrapped("a")).toBe("a"); + + swapImplementation("Free.swappable", (input) => `swapped:${String(input)}`); + try { + expect(wrapped("a")).toBe("swapped:a"); + } finally { + restoreImplementation("Free.swappable"); + } + expect(wrapped("a")).toBe("a"); + }); +}); + function applyMethodDecorator object>( constructor: Class, name: string, diff --git a/test/promotion-applier.test.ts b/test/promotion-applier.test.ts new file mode 100644 index 0000000..4d69ae3 --- /dev/null +++ b/test/promotion-applier.test.ts @@ -0,0 +1,181 @@ +import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { + discoverTrainables, + PromotionRejectedError, + restoreImplementation, + rewritePromotion, + swappedImplementation, + type CandidatePatch, + type ImplementationExecutor, + type PromotionDecision, + type TrainableTarget, +} from "../src/index.js"; + +// The shipped promotion applier, on its own. +// +// `rewritePromotion` does two things at once: it writes the guarded source +// rewrite, and -- for an async target -- it hot-swaps the live implementation +// so a long-running process picks the candidate up without a restart. The +// second half had no test at all. It is the half that changes the behavior of +// an application that is already serving traffic, and the half a rollback has +// to undo as exactly as it undoes the file. +// +// The conformance suite (test/contract.test.ts) checks the applier's refusals; +// these check what it does when it accepts. + +const directory = "test/output/promotion-applier"; + +const source = `class Fixture { + route(input: string): string { + "use training"; + return input; + } + + async slow(input: string): Promise { + "use training"; + return input; + } +} +`; + +let targets: readonly TrainableTarget[]; +let artifact: string; + +beforeEach(async () => { + await rm(directory, { recursive: true, force: true }); + await mkdir(directory, { recursive: true }); + artifact = join(directory, "fixture.ts"); + await writeFile(artifact, source, "utf8"); + targets = discoverTrainables({ files: [artifact] }); +}); + +// The swap registry is module-global and keyed by trainable id, so a test that +// installs one and never rolls back leaves it for the next test in the file -- +// which makes "does not swap" assertions pass or fail on declaration order. +afterEach(() => { + for (const entry of targets) restoreImplementation(entry.id); +}); + +function target(methodName: string): TrainableTarget { + const found = targets.find((entry) => entry.methodName === methodName); + if (!found) throw new Error(`no discovered target named ${methodName}`); + return found; +} + +function candidateFor(methodName: string, implementation: string): CandidatePatch { + return { + id: `candidate-${methodName}`, + trainableId: target(methodName).id, + engineId: "test", + target: target(methodName), + implementation, + }; +} + +function approving(candidate: CandidatePatch): PromotionDecision { + return { candidateId: candidate.id, promote: true, failures: [], meanScore: 1, passRate: 1 }; +} + +/** Records what the applier hands the executor when a swapped call runs. */ +function recordingExecutor(): ImplementationExecutor & { calls: unknown[][] } { + const calls: unknown[][] = []; + const executor = (async (executed, implementation, args, options) => { + calls.push([executed.id, implementation, args, options?.receiver]); + return `executed:${String(args[0])}`; + }) as ImplementationExecutor & { calls: unknown[][] }; + executor.calls = calls; + return executor; +} + +describe("the shipped promotion applier", () => { + it("writes the candidate into the file it was discovered from", async () => { + const candidate = candidateFor("route", "return input.toUpperCase();"); + await rewritePromotion(candidate, approving(candidate)); + + const written = await readFile(artifact, "utf8"); + expect(written).toContain("return input.toUpperCase();"); + expect(written).toContain('"use training";'); + // The sibling method is untouched. + expect(written).toContain("async slow(input: string): Promise {"); + }); + + it("restores the file byte-for-byte on rollback", async () => { + const candidate = candidateFor("route", "return input.toUpperCase();"); + const applied = await rewritePromotion(candidate, approving(candidate)); + await applied.rollback(); + expect(await readFile(artifact, "utf8")).toBe(source); + }); + + describe("hot-swapping an async target", () => { + it("installs a live implementation so a running process picks it up", async () => { + const candidate = candidateFor("slow", "return input.toUpperCase();"); + const executor = recordingExecutor(); + await rewritePromotion(candidate, approving(candidate), executor); + + const swapped = swappedImplementation(candidate.trainableId); + expect(swapped).toBeTypeOf("function"); + }); + + it("routes a swapped call through the executor, forwarding arguments and the receiver", async () => { + const candidate = candidateFor("slow", "return input.toUpperCase();"); + const executor = recordingExecutor(); + await rewritePromotion(candidate, approving(candidate), executor); + + const receiver = { name: "the live instance" }; + const swapped = swappedImplementation(candidate.trainableId); + expect(await swapped?.call(receiver, "abc")).toBe("executed:abc"); + expect(executor.calls).toEqual([[ + candidate.target.id, + "return input.toUpperCase();", + ["abc"], + receiver, + ]]); + }); + + it("removes the live implementation on rollback, along with the file edit", async () => { + const candidate = candidateFor("slow", "return input.toUpperCase();"); + const applied = await rewritePromotion(candidate, approving(candidate), recordingExecutor()); + expect(swappedImplementation(candidate.trainableId)).toBeDefined(); + + await applied.rollback(); + expect(swappedImplementation(candidate.trainableId)).toBeUndefined(); + expect(await readFile(artifact, "utf8")).toBe(source); + }); + }); + + describe("leaving a synchronous target alone", () => { + it("does not swap a sync method, whose calling convention the executor would change", async () => { + // The executor returns a promise. Swapping it in for a method declared + // `(input: string): string` would hand every caller a promise where + // they typed a string, so only async targets swap. + const candidate = candidateFor("route", "return input.toUpperCase();"); + await rewritePromotion(candidate, approving(candidate), recordingExecutor()); + expect(swappedImplementation(candidate.trainableId)).toBeUndefined(); + }); + + it("does not swap an async target when no executor was supplied", async () => { + const candidate = candidateFor("slow", "return input.toUpperCase();"); + await rewritePromotion(candidate, approving(candidate)); + expect(swappedImplementation(candidate.trainableId)).toBeUndefined(); + }); + }); + + describe("refusing to apply", () => { + it.each([ + ["the gate refused", (candidate: CandidatePatch) => ({ ...approving(candidate), promote: false })], + ["the decision names another candidate", () => ({ + candidateId: "someone-else", promote: true, failures: [], meanScore: 1, passRate: 1, + })], + ])("throws PromotionRejectedError and leaves the file alone when %s", async (_label, decide) => { + const candidate = candidateFor("route", "return input.toUpperCase();"); + await expect(rewritePromotion(candidate, decide(candidate) as PromotionDecision)) + .rejects.toBeInstanceOf(PromotionRejectedError); + expect(await readFile(artifact, "utf8")).toBe(source); + expect(swappedImplementation(candidate.trainableId)).toBeUndefined(); + }); + }); +}); From 36a72620efb56097d74fa4e11fd0bc2a4940c7e0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 18:19:34 +0000 Subject: [PATCH 3/6] test: mutate the consent and confinement decisions too Stryker mutated seven modules. Its own stated criterion -- a module where a surviving mutant is alarming rather than merely untidy -- covers three more, each deciding something the library does to a user's machine rather than something it merely computes: - `src/evolve.ts`, the kill switch deciding whether the library may rewrite the user's source without being asked, which must fail closed on a value it does not recognize; - `packages/harness/src/policy.ts`, the sandbox's filesystem and network confinement, where an allowlist failing open would be the worst outcome available; - `packages/rewrite/src/emit.ts`, which appends generated code to the user's own module, where a name that is not an identifier turns a valid file into a syntax error at load time. Adding them left thirteen mutants standing. Four were equivalent: the printer's container name, text, parent linkage and newline kind are all unobservable from the printed string, verified by construction. They are hoisted into named constants and excluded at the line with the reason, which is the escape hatch the threshold note describes -- not a lowered threshold. The other nine were real, and each names a test that was asserting less than it looked like it was: - `isInstrumentable` checked only the method name, so a class name that does not scan as an identifier would be emitted as `owner: () => Not-A-Class`. - `isIdentifierName`'s scanner skipped trivia, so `" normalize"` scanned as a clean identifier. Leading and trailing whitespace, reserved words, member expressions and the empty name now each have a case. - The emitted setter is how a promoted candidate replaces a directive-marked free function, and the test evaluating it used a `wrap` handler returning its own argument -- so it passed whether or not the setter worked. It returns a different function now, and the module's binding is asserted to change. - The evolve switch's message was asserted to match /must be one of/, leaving the list of accepted values -- the entire reason the message exists -- unchecked. Chasing the last of them found a defect repeated six times across two packages. `z.number().int().positive(message)` attaches the message to the positivity check alone, so `minTraces: 2.5`, `fanOut: 2.5`, `maxRounds: 1.5` and `timeoutMs: 1.5` all failed with zod's "Invalid input: expected int, received number", naming neither the setting nor its rule. A `positiveIntegerSetting` helper in each package now carries one message across every constraint, matching what `executionTimeout` already did by hand. Mutation scope: 7 modules -> 10, 348 -> 469 mutants, still under two minutes. Score holds at 100. 813 -> 827 tests. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XRoQAp6kcgCgMvb1jcY115 --- packages/harness/src/policy.ts | 4 +- packages/harness/src/schema.ts | 13 +++++- packages/harness/test/harness.test.ts | 11 ++++- packages/rewrite/src/emit.ts | 16 ++++++-- packages/rewrite/test/instrument.test.ts | 51 ++++++++++++++++++++++-- packages/training/src/errors.ts | 9 +++++ packages/training/src/loop.ts | 6 +-- packages/training/src/training.ts | 3 +- packages/training/test/errors.test.ts | 29 ++++++++++++++ packages/training/test/loop.test.ts | 22 ++++++++++ stryker.config.json | 24 +++++++---- test/tier1.test.ts | 20 ++++++++++ vitest.mutation.config.ts | 4 ++ 13 files changed, 189 insertions(+), 23 deletions(-) diff --git a/packages/harness/src/policy.ts b/packages/harness/src/policy.ts index 4ce5e66..3bfd5a8 100644 --- a/packages/harness/src/policy.ts +++ b/packages/harness/src/policy.ts @@ -3,13 +3,13 @@ import { createRequire } from "node:module"; import type { SandboxPolicy } from "@microsoft/mxc-sdk"; import { z } from "zod"; -import { absolutePath } from "./schema.js"; +import { absolutePath, positiveIntegerSetting } from "./schema.js"; const policySettings = z.object({ workspace: absolutePath, readonlyPaths: z.array(absolutePath).optional(), allowedHosts: z.array(z.string().trim()).optional(), - timeoutMs: z.number().int().positive("timeoutMs must be a positive integer").optional(), + timeoutMs: positiveIntegerSetting("timeoutMs must be a positive integer").optional(), version: z.string().optional(), }); diff --git a/packages/harness/src/schema.ts b/packages/harness/src/schema.ts index a36e321..c432d97 100644 --- a/packages/harness/src/schema.ts +++ b/packages/harness/src/schema.ts @@ -14,7 +14,16 @@ export type MessageKind = z.output; export const messageId = z.string().trim().min(1, "agent message id must be non-empty").brand<"MessageId">(); export type MessageId = z.output; -export const sequenceNumber = z.number().int().positive("sequence must be a positive integer").brand<"SequenceNumber">(); +/** A positive-integer setting whose message holds however the value is wrong. + * `z.number().int().positive(message)` attaches `message` to the positivity + * check alone, so a fractional or non-numeric value failed with zod's generic + * "Invalid input: expected int, received number" -- which never says what the + * setting actually needs. Every constraint carries the same message instead. */ +export function positiveIntegerSetting(message: string) { + return z.number({ error: message }).int(message).positive(message); +} + +export const sequenceNumber = positiveIntegerSetting("sequence must be a positive integer").brand<"SequenceNumber">(); export type SequenceNumber = z.output; export const agentMessage = z.object({ @@ -39,7 +48,7 @@ export const absolutePath = z.string() .brand<"AbsolutePath">(); export type AbsolutePath = z.output; -export const roundLimit = z.number().int().positive("maxRounds must be a positive integer").brand<"RoundLimit">(); +export const roundLimit = positiveIntegerSetting("maxRounds must be a positive integer").brand<"RoundLimit">(); export type RoundLimit = z.output; export const rubricText = z.string().trim().min(1, "judge rubric must be non-empty").brand<"Rubric">(); diff --git a/packages/harness/test/harness.test.ts b/packages/harness/test/harness.test.ts index 8f9223a..66d88cd 100644 --- a/packages/harness/test/harness.test.ts +++ b/packages/harness/test/harness.test.ts @@ -339,11 +339,18 @@ describe("training harness", () => { it.each([ ["a relative workspace", { workspace: "relative/path" }], ["a relative readonly path", { workspace: tmpdir(), readonlyPaths: ["relative/path"] }], - ["a zero timeout", { workspace: tmpdir(), timeoutMs: 0 }], - ["a fractional timeout", { workspace: tmpdir(), timeoutMs: 1.5 }], ])("refuses %s rather than building a policy around it", (_label, settings) => { expect(() => createSandboxPolicy(settings as Parameters[0])).toThrow(); }); + + it.each([ + ["zero", 0], + ["negative", -1], + ["fractional", 1.5], + ])("refuses a %s timeout, saying what a timeout must be", (_label, timeoutMs) => { + expect(() => createSandboxPolicy({ workspace: tmpdir(), timeoutMs })) + .toThrow(/timeoutMs must be a positive integer/); + }); }); it("refuses symlinked paths that resolve outside the workspace", async () => { diff --git a/packages/rewrite/src/emit.ts b/packages/rewrite/src/emit.ts index b960b5d..e7ec356 100644 --- a/packages/rewrite/src/emit.ts +++ b/packages/rewrite/src/emit.ts @@ -46,11 +46,21 @@ export function emitInstrumentation(targets: readonly InstrumentTarget[]): strin [factory.createArrayLiteralExpression(targets.map(entryLiteral), true)], ), ); - const printer = ts.createPrinter({ newLine: ts.NewLineKind.LineFeed }); - const container = ts.createSourceFile("instrumentation.js", "", ts.ScriptTarget.Latest, false, ts.ScriptKind.JS); - return printer.printNode(ts.EmitHint.Unspecified, statement, container); + return printer.printNode(ts.EmitHint.Unspecified, statement, printContainer); } +// The printer and the source file it prints against. Every node here is +// synthesized, so the container's name, text and parent linkage never reach the +// output, and the explicit LineFeed only pins what TypeScript's default already +// resolves to -- belt and braces, so instrumentation appended to an LF file +// stays LF. None of it is observable from the emitted string, so it is excluded +// from mutation rather than pinned by a test that could not tell the +// difference. What the emitted text *is* has its own approved snapshot. +// Stryker disable all +const printer = ts.createPrinter({ newLine: ts.NewLineKind.LineFeed }); +const printContainer = ts.createSourceFile("instrumentation.js", "", ts.ScriptTarget.Latest, false, ts.ScriptKind.JS); +// Stryker restore all + function isInstrumentable(target: InstrumentTarget): boolean { return isIdentifierName(target.methodName) && (target.className === undefined || isIdentifierName(target.className)); } diff --git a/packages/rewrite/test/instrument.test.ts b/packages/rewrite/test/instrument.test.ts index 64f2371..5a89ad0 100644 --- a/packages/rewrite/test/instrument.test.ts +++ b/packages/rewrite/test/instrument.test.ts @@ -105,8 +105,17 @@ describe("instrumentation emission", () => { }); it("registers working accessors when evaluated in the module's scope", () => { + // The `wrap` handler returns a *different* function on purpose. Returning + // the original made the assertions pass whether or not the emitted setter + // worked at all, and rebinding the module's own name is the entire reason + // the setter is emitted -- it is how a promoted candidate replaces a + // directive-marked free function. const { handlers, methods, wrapped } = recordingInstrumentation(); - installInstrumentation(handlers); + const replacement = (input: string) => input.toUpperCase(); + installInstrumentation({ + ...handlers, + wrap: (fn, id) => { wrapped.push(id); return replacement as unknown as typeof fn; }, + }); const evaluate = new Function(`class Router { route(input) { return input; } } function normalize(input) { return input; } ${emitInstrumentation(targets)} @@ -114,10 +123,34 @@ return { Router, normalize };`) as () => { Router: unknown; normalize: unknown } const scope = evaluate(); expect(methods).toEqual([{ owner: scope.Router, methodName: "route", id: "Router.route" }]); expect(wrapped).toEqual(["normalize"]); + expect(scope.normalize).toBe(replacement); }); - it("rejects names that are not plain identifiers", () => { - expect(() => emitInstrumentation([{ id: "bad", methodName: "not a name" }])).toThrow(TypeError); + it("emits the entries one per line, so a rewritten module stays readable", () => { + expect(emitInstrumentation(targets).split("\n")).toHaveLength(4); + }); + + it("appends nothing that would change the file's line endings", () => { + // This text is appended to the user's own module. Emitting CRLF into an + // LF file is diff noise the library has no business creating. + expect(emitInstrumentation(targets)).not.toContain("\r"); + }); + + it("rejects names that are not plain identifiers, naming the offender", () => { + expect(() => emitInstrumentation([{ id: "bad", methodName: "not a name" }])) + .toThrow(/instrument target must be a plain identifier: not a name/); + }); + + it.each([ + ["a leading space", " normalize"], + ["a trailing space", "normalize "], + ["a reserved word", "class"], + ["a member expression", "obj.normalize"], + ["an empty name", ""], + ])("rejects %s as a target name", (_label, methodName) => { + // Anything but a plain identifier would be emitted straight into the + // user's module and turn a valid file into a syntax error at load time. + expect(() => emitInstrumentation([{ id: "bad", methodName }])).toThrow(TypeError); }); }); @@ -150,6 +183,18 @@ describe("createRewriter", () => { expect(createRewriter(() => [computed], "use audit")(marked, "/app/f.js")).toBe(marked); }); + it("skips a target whose class name cannot be referenced, keeping the method's own name valid", () => { + // Only the method name was ever checked here. A class name that does not + // scan as an identifier would be emitted as `owner: () => Not-A-Class`, + // which is a syntax error in the file the library just rewrote. + const marked = '"use audit"; class C { m() {} }'; + const bad: InstrumentTarget = { id: "weird", methodName: "m", className: "Not-A-Class" }; + expect(createRewriter(() => [bad], "use audit")(marked, "/app/c.js")).toBe(marked); + + const good: InstrumentTarget = { id: "C.m", methodName: "m", className: "C" }; + expect(createRewriter(() => [good], "use audit")(marked, "/app/c.js")).not.toBe(marked); + }); + it("only accepts \"use \" markers", () => { // @ts-expect-error markers must be `use ${string}` directives expect(() => createRewriter(() => [], "audit")).toThrow(TypeError); diff --git a/packages/training/src/errors.ts b/packages/training/src/errors.ts index f778d31..4193f51 100644 --- a/packages/training/src/errors.ts +++ b/packages/training/src/errors.ts @@ -260,6 +260,15 @@ export class InvalidSettingsError extends TsAutocodeTypeError { /** Parse with a Zod schema, surfacing failures as `InvalidSettingsError` * instead of leaking `ZodError` to consumers. The first issue's message is used * verbatim, so the schemas' hand-written messages still read as before. */ +/** A positive-integer setting whose message holds however the value is wrong. + * `z.number().int().positive(message)` attaches `message` to the positivity + * check alone, so a fractional or non-numeric value failed with zod's generic + * "Invalid input: expected int, received number" -- which never says what the + * setting actually needs. Every constraint carries the same message instead. */ +export function positiveIntegerSetting(message: string) { + return z.number({ error: message }).int(message).positive(message); +} + export function parseSetting(schema: z.ZodType, value: unknown): T { const result = schema.safeParse(value); if (result.success) return result.data; diff --git a/packages/training/src/loop.ts b/packages/training/src/loop.ts index ca4bd20..8564a22 100644 --- a/packages/training/src/loop.ts +++ b/packages/training/src/loop.ts @@ -1,7 +1,7 @@ import { z } from "zod"; import type { CandidatePatch } from "./engine.js"; -import { parseSetting } from "./errors.js"; +import { parseSetting, positiveIntegerSetting } from "./errors.js"; import type { TrainableEvalRun } from "./evaluation.js"; import type { PromotionDecision } from "./promotion.js"; import type { TrainableId } from "./token.js"; @@ -87,8 +87,8 @@ export interface RoundSequence { * in order; within a round up to `fanOut` candidate pipelines run * concurrently. A round that reviews nothing new (every slot proposed an * already-seen candidate) completes the sequence as `"stalled"`. */ -const roundLimit = z.number().int().positive("maxRounds must be a positive integer"); -const fanOutWidth = z.number().int().positive("fanOut must be a positive integer"); +const roundLimit = positiveIntegerSetting("maxRounds must be a positive integer"); +const fanOutWidth = positiveIntegerSetting("fanOut must be a positive integer"); export function trainingRounds(input: TrainingLoopInput): RoundSequence { const maxRounds = parseSetting(roundLimit, input.maxRounds ?? defaultMaxRounds); diff --git a/packages/training/src/training.ts b/packages/training/src/training.ts index acacbb1..b2ae7f7 100644 --- a/packages/training/src/training.ts +++ b/packages/training/src/training.ts @@ -12,6 +12,7 @@ import { ExecutorNotConfiguredError, InsufficientTracesError, parseSetting, + positiveIntegerSetting, PromotionApplierNotConfiguredError, PromotionRejectedError, TraceNotFoundError, @@ -52,7 +53,7 @@ import { const trainableAttribute = "ts_autocode.trainable.id"; const tracerName = "ts-autocode"; -const traceMinimum = z.number().int().positive("minTraces must be a positive integer"); +const traceMinimum = positiveIntegerSetting("minTraces must be a positive integer"); const executionTimeout = z.number().positive("execution.timeoutMs must be a positive number of milliseconds").finite("execution.timeoutMs must be a positive number of milliseconds"); export interface CaptureSettings { diff --git a/packages/training/test/errors.test.ts b/packages/training/test/errors.test.ts index faedfc6..f31a3a6 100644 --- a/packages/training/test/errors.test.ts +++ b/packages/training/test/errors.test.ts @@ -15,6 +15,7 @@ import { MissingSecretError, OperationInterruptedError, parseSetting, + positiveIntegerSetting, PromotionApplierNotConfiguredError, PromotionRejectedError, SourceDiscoveryError, @@ -180,3 +181,31 @@ describe("parseSetting", () => { expect(parseSetting(z.string().transform((value) => value.length), "abcd")).toBe(4); }); }); + +describe("positiveIntegerSetting", () => { + // `z.number().int().positive(message)` attaches the message to the + // positivity check alone, so every setting written that way reported zod's + // "Invalid input: expected int, received number" for a fractional value -- + // a message that names neither the setting nor what it needs. Six settings + // across two packages were written that way. This is the shape that fixes + // them, so the message has to hold for every way the value can be wrong. + const schema = positiveIntegerSetting("minTraces must be a positive integer"); + + it("accepts a positive integer", () => { + expect(parseSetting(schema, 3)).toBe(3); + }); + + it.each([ + ["zero", 0], + ["negative", -1], + ["fractional", 2.5], + ["a negative fraction", -0.5], + ["Infinity", Number.POSITIVE_INFINITY], + ["NaN", Number.NaN], + ["a string", "many"], + ["null", null], + ["undefined", undefined], + ])("reports the same message for %s", (_label, value) => { + expect(() => parseSetting(schema, value)).toThrow("minTraces must be a positive integer"); + }); +}); diff --git a/packages/training/test/loop.test.ts b/packages/training/test/loop.test.ts index 4dedd78..1d85f57 100644 --- a/packages/training/test/loop.test.ts +++ b/packages/training/test/loop.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest"; import { createCandidateReview, createPromotionDecision, + defineTrainable, sequentialLoop, trainingRounds, type CandidatePatch, @@ -116,6 +117,27 @@ describe("training loop fan-out", () => { }); }); +describe("round settings", () => { + // A fractional round or fan-out count used to fail with zod's generic + // "expected int, received number", naming neither the setting nor its rule. + it.each([ + ["maxRounds", { maxRounds: 1.5 }, "maxRounds must be a positive integer"], + ["maxRounds at zero", { maxRounds: 0 }, "maxRounds must be a positive integer"], + ["fanOut", { fanOut: 2.5 }, "fanOut must be a positive integer"], + ["fanOut at zero", { fanOut: 0 }, "fanOut must be a positive integer"], + ])("refuses a bad %s, naming the setting and its rule", async (_label, overrides, message) => { + await expect(sequentialLoop({ + trainableId: defineTrainable("Router.route").id, + objective: "improve", + rubric: "must pass", + outputDir: "test/output/loop-settings", + propose: async () => { throw new Error("never proposed"); }, + review: async () => { throw new Error("never reviewed"); }, + ...overrides, + })).rejects.toThrow(message); + }); +}); + describe("observable round sequence", () => { it("emits each reviewed round in order before completing", async () => { const emitted: Array = []; diff --git a/stryker.config.json b/stryker.config.json index 60fc2f9..ffc2c26 100644 --- a/stryker.config.json +++ b/stryker.config.json @@ -24,7 +24,10 @@ "packages/training/src/digest.ts", "packages/training/src/builders.ts", "packages/rewrite/src/apply.ts", - "packages/rewrite/src/canonical.ts" + "packages/rewrite/src/canonical.ts", + "packages/rewrite/src/emit.ts", + "packages/harness/src/policy.ts", + "src/evolve.ts" ], "thresholds": { "high": 100, @@ -32,12 +35,19 @@ "break": 100 }, "_comment": [ - "Scoped to the modules where a surviving mutant is alarming rather than merely untidy:", - "the promotion gate decides whether generated code is written to a user's source file,", - "and the rewrite guard decides whether it lands on the body it was verified against.", - "A mutant that survives in either means a test asserts the happy path without pinning", - "the decision. Mutating everything would take hours and mostly re-measure line coverage,", - "which vitest already enforces.", + "Scoped to the modules where a surviving mutant is alarming rather than merely untidy.", + "Each one decides something the library does to a user's machine, not merely something", + "it computes: the promotion gate decides whether generated code is written to a source", + "file at all; the rewrite guard decides whether it lands on the body it was verified", + "against; evolve.ts is the kill switch deciding whether the library may rewrite that", + "source without being asked, and must fail closed on a value it does not recognize;", + "policy.ts is the sandbox's filesystem and network confinement, where the allowlist", + "failing open would be the worst outcome available; emit.ts appends generated code to", + "the user's own module, where a name that is not an identifier turns a valid file into", + "a syntax error at load time.", + "A mutant that survives in any of these means a test asserts the happy path without", + "pinning the decision. Mutating everything would take hours and mostly re-measure line", + "coverage, which vitest already enforces.", "The break threshold is a ratchet, like the coverage thresholds: raise it as suites", "improve, never lower it to get a build green. It reached 100 by killing the last", "survivors in the rewrite formatter; a genuinely equivalent mutant -- one no test", diff --git a/test/tier1.test.ts b/test/tier1.test.ts index 9543178..c160a87 100644 --- a/test/tier1.test.ts +++ b/test/tier1.test.ts @@ -46,6 +46,26 @@ describe("evolution kill switch", () => { expect(() => evolutionEnabled("nope")).toThrow(evolveVariable); expect(() => evolutionEnabled("maybe")).toThrow(/must be one of/); }); + + it("lists every value it would have accepted, and quotes the one it got", () => { + // Failing closed is only half of it: a user who typed the wrong thing has + // to be told what the right thing is, or the kill switch is a dead end. + // Asserting only /must be one of/ let the list itself go unchecked. + const message = (() => { + try { + evolutionEnabled("nope"); + } catch (error) { + return (error as Error).message; + } + throw new Error("evolutionEnabled accepted an unrecognized value"); + })(); + + for (const accepted of ["1", "true", "on", "yes", "enabled", "0", "false", "off", "no", "disabled"]) { + expect(message).toContain(accepted); + } + expect(message).toContain(", "); + expect(message).toContain('received "nope"'); + }); }); describe("promotion thresholds", () => { diff --git a/vitest.mutation.config.ts b/vitest.mutation.config.ts index a203162..43c29c2 100644 --- a/vitest.mutation.config.ts +++ b/vitest.mutation.config.ts @@ -31,6 +31,10 @@ export default defineConfig({ "packages/training/test/builders.test.ts", "packages/rewrite/test/apply.test.ts", "packages/rewrite/test/canonical.test.ts", + "packages/rewrite/test/instrument.test.ts", + "packages/harness/test/harness.test.ts", + "test/tier1.test.ts", + "test/register.test.ts", ], }, }); From 9c434ca61d79418d8cce1519647c93d84d7e3484 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 18:45:43 +0000 Subject: [PATCH 4/6] fix: build the sibling packages before mutating The mutation job in CI ran `stryker run` against a tree that had never been built. Several of the tests it runs import a sibling package by name -- `packages/training/test/promotion.test.ts` imports ts-autocode-rewrite -- which resolves through that package's `dist/`. Without it those files fail to collect, and Stryker scores every mutant they would have killed as a survivor rather than reporting an error. So the promotion gate, the module whose whole reason for being in the mutated set is that it decides whether generated code reaches a user's file, was being graded in CI with its own test file not running. Four mutants stood there, including one that blanks the message telling a user what range `minScore` accepts. At the old threshold of 95 the shortfall to 98.85 was invisible, and had been. Raising it to 100 is what surfaced it -- which is the other thing a threshold of 100 buys: it notices a test file quietly dropping out of the run. `test:mutation` now builds what it needs, rather than CI having to remember to. `build:packages` factors the four sibling builds that `build` and `typecheck` already ran in sequence. Verified from a clean tree with every `dist/` removed: 100, no survivors. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XRoQAp6kcgCgMvb1jcY115 --- package.json | 9 +++++---- stryker.config.json | 7 ++++++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index b8fc7a9..5a6f441 100644 --- a/package.json +++ b/package.json @@ -54,18 +54,19 @@ "url": "https://github.com/Tyler-R-Kendrick/ts-autocode/issues" }, "scripts": { - "build": "npm run build:grounding && npm run build:harness && npm run build:rewrite && npm run build:training && npm run build:core", + "build": "npm run build:packages && npm run build:core", "build:grounding": "node -e \"require('node:fs').rmSync('packages/grounding/dist', { recursive: true, force: true })\" && tsc -p packages/grounding/tsconfig.json", "build:core": "node -e \"require('node:fs').rmSync('dist', { recursive: true, force: true })\" && tsc -p tsconfig.json", "build:harness": "node -e \"require('node:fs').rmSync('packages/harness/dist', { recursive: true, force: true })\" && tsc -p packages/harness/tsconfig.json", - "typecheck": "npm run build:grounding && npm run build:harness && npm run build:rewrite && npm run build:training && tsc --noEmit -p tsconfig.test.json && tsc --noEmit -p packages/grounding/tsconfig.test.json && tsc --noEmit -p packages/harness/tsconfig.test.json && tsc --noEmit -p packages/rewrite/tsconfig.test.json && tsc --noEmit -p packages/training/tsconfig.test.json", + "typecheck": "npm run build:packages && tsc --noEmit -p tsconfig.test.json && tsc --noEmit -p packages/grounding/tsconfig.test.json && tsc --noEmit -p packages/harness/tsconfig.test.json && tsc --noEmit -p packages/rewrite/tsconfig.test.json && tsc --noEmit -p packages/training/tsconfig.test.json", "test": "node test/run.mjs", "test:coverage": "node test/run.mjs --coverage", - "test:mutation": "stryker run", + "test:mutation": "npm run build:packages && stryker run", "check": "npm run typecheck && npm run test:coverage && npm run build:core", "prepublishOnly": "npm run check", "build:training": "node -e \"require('node:fs').rmSync('packages/training/dist', { recursive: true, force: true })\" && tsc -p packages/training/tsconfig.json", - "build:rewrite": "node -e \"require('node:fs').rmSync('packages/rewrite/dist', { recursive: true, force: true })\" && tsc -p packages/rewrite/tsconfig.json" + "build:rewrite": "node -e \"require('node:fs').rmSync('packages/rewrite/dist', { recursive: true, force: true })\" && tsc -p packages/rewrite/tsconfig.json", + "build:packages": "npm run build:grounding && npm run build:harness && npm run build:rewrite && npm run build:training" }, "keywords": [ "agentv", diff --git a/stryker.config.json b/stryker.config.json index 60fc2f9..ec58ffd 100644 --- a/stryker.config.json +++ b/stryker.config.json @@ -42,7 +42,12 @@ "improve, never lower it to get a build green. It reached 100 by killing the last", "survivors in the rewrite formatter; a genuinely equivalent mutant -- one no test", "could distinguish -- is excluded at the line with a `// Stryker disable next-line", - ": ` comment, which is reviewable, rather than by lowering this." + ": ` comment, which is reviewable, rather than by lowering this.", + "Holding it at 100 is also the only thing that notices a test file dropping out of", + "the run. `npm run test:mutation` builds the sibling packages first because several", + "of these tests import them by name, through their dist/; without that build the", + "files fail to collect, and Stryker scores their mutants as survivors rather than", + "as an error. At a threshold of 95 that was invisible, and had been for a while." ], "ignorePatterns": [ "test/output", From cdd75cc8c5e37b4d639308550882a7fe629da3d9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 18:54:11 +0000 Subject: [PATCH 5/6] fix: seed the fuzz corpus meta-tests `check (20)` failed on a corpus that was doing its job: "produces modules that discovery actually finds targets in" drew 79 against a threshold of 80. The three tests at the bottom of the fuzz suite measure the *generator* rather than the code, but drew unseeded, so each run saw a different corpus. `markedModule` can emit a class of only unmarked methods and no marked free function, which puts the hit rate at a binomial around 90 in 100 -- measured across seeds it ranges 83 to 95, so a threshold of 80 sits about three standard deviations out and fails roughly once in a few hundred runs. It has been that way since the corpus tests were written; this run is the first to draw the tail. Raising the threshold would trade one flake for a looser assertion. A fixed seed measures the same corpus every time and on every machine, and still fails loudly if the generators change such that the corpus stops reaching real code -- which is the entire point of these three. The properties above them are untouched and still explore. The damaged-module case gains an upper bound while it is here: it asserts the corpus is a genuine mix, and only had the lower half of that. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XRoQAp6kcgCgMvb1jcY115 --- test/fuzz.test.ts | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/test/fuzz.test.ts b/test/fuzz.test.ts index f19271f..27cee6c 100644 --- a/test/fuzz.test.ts +++ b/test/fuzz.test.ts @@ -206,10 +206,22 @@ describe("the fuzz corpus itself", () => { // A corpus that never reaches the code under test makes every property // above vacuously true. This asserts the corpus does its job, so the suite // cannot quietly decay into theatre. + // + // Unlike the properties above, these three measure the *generator* rather + // than the code, so they draw from a fixed seed. Unseeded, each run drew a + // different corpus: `markedModule` can emit a class of only unmarked + // methods and no marked free function, so the hit rate is a binomial around + // 90 in 100 -- measured across seeds it ranges 83 to 95 -- and a threshold + // of 80 sits about three standard deviations out. CI duly drew 79 one run + // and failed on a corpus that was doing its job. A seeded draw measures the + // same thing every time and on every machine, and still fails loudly if the + // generators change such that the corpus stops reaching real code, which is + // the whole point of these three. + const corpusSeed = 1; + it("produces modules that discovery actually finds targets in", () => { let withTargets = 0; - const samples = fc.sample(markedModule, 100); - for (const source of samples) { + for (const source of fc.sample(markedModule, { numRuns: 100, seed: corpusSeed })) { if (discoverInSource(source, "fuzz.ts").length > 0) withTargets += 1; } expect(withTargets).toBeGreaterThan(80); @@ -217,16 +229,17 @@ describe("the fuzz corpus itself", () => { it("produces damaged modules that still often parse", () => { let withTargets = 0; - for (const source of fc.sample(damagedModule, 200)) { + for (const source of fc.sample(damagedModule, { numRuns: 200, seed: corpusSeed })) { if (safeDiscover(source).length > 0) withTargets += 1; } // Damaged input should be a genuine mix, not all-or-nothing. expect(withTargets).toBeGreaterThan(10); + expect(withTargets).toBeLessThan(200); }); it("produces modules the load hook actually rewrites", () => { let rewritten = 0; - for (const source of fc.sample(markedModule, 100)) { + for (const source of fc.sample(markedModule, { numRuns: 100, seed: corpusSeed })) { if (augmentSource(source, "fuzz.ts") !== source) rewritten += 1; } expect(rewritten).toBeGreaterThan(80); From e46987cb1ddfba4f25c01335be624128a0b3b1f9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 19:01:27 +0000 Subject: [PATCH 6/6] fix: restore parseSetting's doc comment Self-review: `positiveIntegerSetting` went in between `parseSetting`'s JSDoc and `parseSetting` itself, so a public export lost its documentation and an orphaned comment block sat above the new helper. Both copies now also say they are copies, matching how `attempt`/`errorMessage` document the same deliberate duplication. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XRoQAp6kcgCgMvb1jcY115 --- packages/harness/src/schema.ts | 6 +++++- packages/training/src/errors.ts | 12 ++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/harness/src/schema.ts b/packages/harness/src/schema.ts index c432d97..a448a90 100644 --- a/packages/harness/src/schema.ts +++ b/packages/harness/src/schema.ts @@ -18,7 +18,11 @@ export type MessageId = z.output; * `z.number().int().positive(message)` attaches `message` to the positivity * check alone, so a fractional or non-numeric value failed with zod's generic * "Invalid input: expected int, received number" -- which never says what the - * setting actually needs. Every constraint carries the same message instead. */ + * setting actually needs. Every constraint carries the same message instead. + * + * Deliberately duplicated in packages/training/src/errors.ts rather than shared + * through a new package for three lines, as `attempt`/`errorMessage` are; keep + * the copies identical. */ export function positiveIntegerSetting(message: string) { return z.number({ error: message }).int(message).positive(message); } diff --git a/packages/training/src/errors.ts b/packages/training/src/errors.ts index 4193f51..2ee797e 100644 --- a/packages/training/src/errors.ts +++ b/packages/training/src/errors.ts @@ -257,18 +257,22 @@ export class InvalidSettingsError extends TsAutocodeTypeError { } } -/** Parse with a Zod schema, surfacing failures as `InvalidSettingsError` - * instead of leaking `ZodError` to consumers. The first issue's message is used - * verbatim, so the schemas' hand-written messages still read as before. */ /** A positive-integer setting whose message holds however the value is wrong. * `z.number().int().positive(message)` attaches `message` to the positivity * check alone, so a fractional or non-numeric value failed with zod's generic * "Invalid input: expected int, received number" -- which never says what the - * setting actually needs. Every constraint carries the same message instead. */ + * setting actually needs. Every constraint carries the same message instead. + * + * Deliberately duplicated in packages/harness/src/schema.ts rather than shared + * through a new package for three lines, as `attempt`/`errorMessage` are; keep + * the copies identical. */ export function positiveIntegerSetting(message: string) { return z.number({ error: message }).int(message).positive(message); } +/** Parse with a Zod schema, surfacing failures as `InvalidSettingsError` + * instead of leaking `ZodError` to consumers. The first issue's message is used + * verbatim, so the schemas' hand-written messages still read as before. */ export function parseSetting(schema: z.ZodType, value: unknown): T { const result = schema.safeParse(value); if (result.success) return result.data;