Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion docs/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ land, never lower them to get a build green.
| Surface | `test/surface.test.ts` | Re-export drift between the root package and its siblings |
| Protocol | `test/digest-protocol.test.ts` | Two packages that must agree without importing each other |
| Compatibility | `test/deprecated.test.ts` | A deprecated spelling that silently stopped working |
| Property | `test/property.test.ts` | A law that holds for chosen examples but not in general |
| Fuzz | `test/fuzz.test.ts` | A parser crashing, hanging, or corrupting source it did not write |
| Characterization | `test/characterization*.test.ts` | A change to anything this library *generates* — rewritten source, emitted instrumentation, prompts, CLI output, the export surface |

## Running
Expand Down Expand Up @@ -52,7 +54,25 @@ Snapshots are excluded from `tsconfig.test.json`: they are generated artifacts,
and the emitted instrumentation deliberately references names from the module it
is appended to, so it does not typecheck standalone.

## Conventions
## Property and fuzz tests

`fast-check` states the law and hunts for a counterexample, rather than sampling
a few inputs by hand. Failures print a shrunk counterexample and a seed, so a
regression is reproducible rather than "it failed once on CI".

Properties target the pure, total functions where examples can only sample:
identity round-trips, digest canonicalization, gate aggregation, and the spread
helpers. Fuzzing targets the parsers, because every one of them runs against
code this library did not write — `augmentSource` sees every module a user
loads.

**A fuzz corpus must reach the code.** An early version used random punctuation;
instrumenting it showed **1 input in 3000** produced a discovered target, so
every property about offsets and rewriting was passing vacuously.
`test/support/sources.ts` now generates structurally plausible marked modules
and then damages them, and `test/fuzz.test.ts` asserts the corpus still reaches
real work — so the suite cannot quietly decay back into theatre.


- **A test names the defect it prevents.** Where a test exists because
something was once wrong, the comment says what was wrong. That is what makes
Expand Down
2 changes: 1 addition & 1 deletion packages/training/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export type {
TrainingRound,
} from "./loop.js";

export { defineTrainable, trainableTokenFromSymbol } from "./token.js";
export { defineTrainable, toTrainableToken, trainableTokenFromSymbol } from "./token.js";
export type { TrainableId, TrainableIdentity, TrainableToken } from "./token.js";

export { defaultTsconfig, discoverInSource, discoverTrainables, inMemoryArtifactRef, trainingMarker } from "./source.js";
Expand Down
13 changes: 8 additions & 5 deletions packages/training/src/optional.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,12 @@ export function optional<K extends string, V>(key: K, value: V | undefined): { [
/** Spreads every defined entry of `values`, dropping the undefined ones.
* `{ ...defined({ signal, timeoutMs }) }` replaces a run of `optional` calls. */
export function defined<T extends object>(values: T): { [K in keyof T]?: Exclude<T[K], undefined> } {
const result: Record<string, unknown> = {};
for (const [key, value] of Object.entries(values)) {
if (value !== undefined) result[key] = value;
}
return result as { [K in keyof T]?: Exclude<T[K], undefined> };
// `Object.fromEntries`, not `result[key] = value`. Assignment goes through
// the `__proto__` setter on Object.prototype, so a `__proto__` key was
// silently dropped -- and with an object value it replaced the result's
// prototype instead of adding a key. `fromEntries` defines own properties,
// which is what a key/value copy should do. Found by a property test.
return Object.fromEntries(
Object.entries(values).filter(([, value]) => value !== undefined),
) as { [K in keyof T]?: Exclude<T[K], undefined> };
}
11 changes: 9 additions & 2 deletions packages/training/src/promotion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,15 @@ import { z } from "zod";
import type { BoundEvaluation, CandidatePatch } from "./engine.js";
import { parseSetting } from "./errors.js";

const unitInterval = (name: string) =>
z.number().finite(`${name} must be between 0 and 1`).min(0, `${name} must be between 0 and 1`).max(1, `${name} must be between 0 and 1`);
/** A threshold in [0, 1]. Every rejection -- wrong type, NaN, Infinity, or
* merely out of range -- reports the same message, because a user who passed a
* bad threshold wants to know the range, not Zod's type vocabulary. Without the
* base-schema message, `minScore: Infinity` reported "expected number,
* received number", which says nothing useful. */
const unitInterval = (name: string) => {
const message = `${name} must be between 0 and 1`;
return z.number({ error: message }).finite(message).min(0, message).max(1, message);
};
const minScoreThreshold = unitInterval("minScore");
const minPassRateThreshold = unitInterval("minPassRate");

Expand Down
20 changes: 18 additions & 2 deletions packages/training/src/source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,19 @@ export interface TrainableTarget {
readonly parameters: readonly TrainableParameter[];
readonly returnType: string;
readonly async: boolean;
/** Offset of the first character after the directive (or after the body's
* opening brace when the marker is a decorator). Guarded rewriting replaces
* exactly `[bodyStart, bodyEnd)`. */
readonly bodyStart: number;
/** Offset of the body's closing brace. */
readonly bodyEnd: number;
/** Digest of the **raw** `[bodyStart, bodyEnd)` slice, whitespace included.
* This is what guarded application re-computes to decide whether the file
* changed since discovery, so it must not be derived from
* {@link TrainableTarget.implementation}, which is trimmed. */
readonly bodyDigest: string;
/** The body as an engine should read it: the same slice, **trimmed**. The
* untrimmed slice is recoverable as `source.slice(bodyStart, bodyEnd)`. */
readonly implementation: string;
readonly indentation: string;
}
Expand Down Expand Up @@ -162,8 +172,14 @@ function targetFor(
if (node.asteriskToken) throw new InvalidTrainableIdentityError(`generator methods cannot be trainable: ${artifactRef}`);
const body = node.body as ts.Block;
const directive = firstDirective(body);
const bodyStart = directive?.end ?? body.getStart(sourceFile) + 1;
const bodyEnd = body.end - 1;
// TypeScript's error recovery synthesizes a body for an unterminated block,
// whose `end` can sit past EOF -- so a truncated file yielded a target
// claiming offsets outside its own source. Slicing clamps, so nothing was
// corrupted, but publishing an out-of-range range is malformed data crossing
// a public boundary. Clamp it: a no-op for source that parses.
const limit = sourceFile.text.length;
const bodyStart = Math.min(directive?.end ?? body.getStart(sourceFile) + 1, limit);
const bodyEnd = Math.max(bodyStart, Math.min(body.end - 1, limit));
const implementation = sourceFile.text.slice(bodyStart, bodyEnd);
const methodName = node.name?.getText(sourceFile) ?? "anonymous";
const parameters = node.parameters.map((parameter, index): TrainableParameter => ({
Expand Down
41 changes: 39 additions & 2 deletions packages/training/test/optional.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@ describe("optional", () => {
it("omits the key entirely when the value is undefined", () => {
const spread = { ...optional("signal", undefined) };
expect(Object.keys(spread)).toEqual([]);
expect("signal" in spread).toBe(false);
// `Object.hasOwn`, not `in`: `in` walks the prototype chain.
expect(Object.hasOwn(spread, "signal")).toBe(false);
});

it("keeps falsy-but-defined values, which is the whole point", () => {
for (const value of [0, "", false, Number.NaN, null] as const) {
const spread = { ...optional("v", value) };
expect("v" in spread).toBe(true);
expect(Object.hasOwn(spread, "v")).toBe(true);
expect((spread as { v: unknown }).v).toBe(value === value ? value : (spread as { v: number }).v);
}
expect(Number.isNaN(({ ...optional("v", Number.NaN) } as { v: number }).v)).toBe(true);
Expand Down Expand Up @@ -70,3 +71,39 @@ describe("defined", () => {
expect({ ...defined(base) }).toEqual({ own: "yes" });
});
});

describe("keys that shadow Object.prototype", () => {
// `defined` built its result with `result[key] = value`, which goes through
// the `__proto__` setter: a `__proto__` key was silently dropped, and with
// an object value it replaced the result's prototype instead of adding a
// key. Found by a property test over arbitrary dictionaries.
const dangerous = ["__proto__", "constructor", "prototype", "toString", "hasOwnProperty", "valueOf"];

it.each(dangerous)("defined keeps a %s key as an own property", (key) => {
const result = defined(JSON.parse(`{"${key}": 1}`) as Record<string, unknown>);
expect(Object.hasOwn(result, key)).toBe(true);
expect((result as Record<string, unknown>)[key]).toBe(1);
});

it.each(dangerous)("optional keeps a %s key as an own property", (key) => {
const result = { ...optional(key, 1) };
expect(Object.hasOwn(result, key)).toBe(true);
});

it("never lets a __proto__ value replace the result's prototype", () => {
const result = defined(JSON.parse('{"__proto__": {"isAdmin": true}}') as Record<string, unknown>);
expect(Object.getPrototypeOf(result)).toBe(Object.prototype);
expect((result as Record<string, unknown>)["isAdmin"]).toBeUndefined();
expect(Object.getPrototypeOf({ ...optional("__proto__", { isAdmin: true }) })).toBe(Object.prototype);
});

it("does not pollute Object.prototype itself", () => {
defined(JSON.parse('{"__proto__": {"polluted": true}}') as Record<string, unknown>);
expect(({} as Record<string, unknown>)["polluted"]).toBeUndefined();
});

it("still drops an undefined value under a dangerous key", () => {
const result = defined({ ["__proto__"]: undefined } as Record<string, unknown>);
expect(Object.getOwnPropertyNames(result)).toEqual([]);
});
});
52 changes: 52 additions & 0 deletions packages/training/test/promotion.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest";
import { commitRewrite, revertRewrite } from "ts-autocode-rewrite";

import {
defaultMinScore,
defineTrainable,
evaluatePromotionGate,
type CandidatePatch,
Expand Down Expand Up @@ -105,3 +106,54 @@ describe("promotion", () => {
expect(decision.failures).toContain("AgentV evaluations must be run against the candidate");
});
});

describe("threshold validation messages", () => {
// A user who passes a bad threshold wants to know the range, not Zod's type
// vocabulary. `minScore: Infinity` used to report "expected number, received
// number", because the base schema rejected it before the .finite() message
// could apply. Found by a property test over non-unit-interval doubles.
const bad: ReadonlyArray<readonly [string, unknown]> = [
["negative", -0.5],
["above one", 1.5],
["positive infinity", Number.POSITIVE_INFINITY],
["negative infinity", Number.NEGATIVE_INFINITY],
["NaN", Number.NaN],
["a string", "0.5"],
["a boolean", true],
["an object", {}],
];

it.each(bad)("reports the range for %s", async (_label, value) => {
const promise = evaluatePromotionGate({
candidate: candidate(), evaluations: [], conformance: true, minScore: value as number,
});
await expect(promise).rejects.toThrow("minScore must be between 0 and 1");
});

it("names the offending setting, not a generic one", async () => {
await expect(evaluatePromotionGate({
candidate: candidate(), evaluations: [], conformance: true, minPassRate: 2,
})).rejects.toThrow("minPassRate must be between 0 and 1");
});

it("treats null and undefined as unset, applying the default", async () => {
// `minScore ?? defaultMinScore` coalesces both. Neither is type-valid, so
// this pins the behavior rather than endorsing it as an input.
for (const value of [undefined, null]) {
// `exactOptionalPropertyTypes` forbids passing an explicit undefined,
// which is the point: neither spelling is type-valid, so this pins
// runtime behavior rather than endorsing either as an input.
const input = { candidate: candidate(), evaluations: [], conformance: true, minScore: value };
const decision = await evaluatePromotionGate(input as unknown as Parameters<typeof evaluatePromotionGate>[0]);
expect(decision.failures.some((failure) => failure.includes(`below ${defaultMinScore}`))).toBe(true);
}
});

it("accepts the closed interval's endpoints", async () => {
for (const value of [0, 1]) {
await expect(evaluatePromotionGate({
candidate: candidate(), evaluations: [], conformance: true, minScore: value, minPassRate: value,
})).resolves.toBeDefined();
}
});
});
28 changes: 28 additions & 0 deletions packages/training/test/source.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,31 @@ describe("parameter types inferred from literal defaults", () => {
expect(declare("value")[0]?.type).toBe("unknown");
});
});

describe("offsets on source TypeScript could not fully parse", () => {
// Error recovery synthesizes a body for an unterminated block whose `end`
// can sit past EOF, so a truncated file produced a target claiming offsets
// outside its own source. Found by fuzzing generated-then-damaged modules.
const truncated = 'class Router {\n\troute(input: string): string {\n\t\t"use training";\n\t\t// } a brace in a comm';

it("never reports offsets outside the source", () => {
for (const target of discoverInSource(truncated, "truncated.ts")) {
expect(target.bodyStart).toBeGreaterThanOrEqual(0);
expect(target.bodyEnd).toBeLessThanOrEqual(truncated.length);
expect(target.bodyStart).toBeLessThanOrEqual(target.bodyEnd);
}
});

it("keeps the digest consistent with the clamped slice", () => {
for (const target of discoverInSource(truncated, "truncated.ts")) {
const raw = truncated.slice(target.bodyStart, target.bodyEnd);
expect(raw.trim()).toBe(target.implementation);
}
});

it("leaves offsets untouched for source that parses", () => {
const valid = 'class Router {\n\troute(input: string): string {\n\t\t"use training";\n\t\treturn input;\n\t}\n}\n';
const target = discoverInSource(valid, "valid.ts")[0]!;
expect(valid.slice(target.bodyStart, target.bodyEnd).trim()).toBe("return input;");
});
});
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export {
resetTraining,
sequentialLoop,
SourceDiscoveryError,
toTrainableToken,
trainableTokenFromSymbol,
training,
TraceNotFoundError,
Expand Down
Loading
Loading