diff --git a/.gitignore b/.gitignore index 7f2ae85..fbfa053 100644 --- a/.gitignore +++ b/.gitignore @@ -28,3 +28,7 @@ Thumbs.db # They go stale the moment code merges; the durable "why" lives in docs/specs/ + docs/adrs/. docs/plans/* !docs/plans/README.md + +# E2 writes each run's measured table here; experiments/RESULTS.md is the committed +# baseline and is no longer overwritten by a test run (see experiments/README.md). +experiments/.results/ diff --git a/experiments/README.md b/experiments/README.md index 9fce169..d460dd1 100644 --- a/experiments/README.md +++ b/experiments/README.md @@ -15,11 +15,35 @@ docker run -d --rm --name sh-m6-redis -p 6379:6379 redis:7-alpine pnpm -C experiments test ``` -- E2 writes recorded results to `experiments/RESULTS.md` and asserts the - checkpoint/backend read ratio grows with session length. +- E2 asserts the checkpoint/backend read ratio grows with session length, writes each + run's measured table to the gitignored `experiments/.results/RESULTS.md`, and checks the + reproducible columns against the committed baseline in `experiments/RESULTS.md`. - E5 structural asserts the voter blocks + persists exactly one `abort` over cap, and is inert when the cap is disabled. +### The E2 baseline + +`experiments/RESULTS.md` is committed and is **not** rewritten by a test run — that used to +leave the working tree dirty with machine-local timings after any `pnpm -r test`. A run now +writes its own table to the gitignored `experiments/.results/` and compares only the columns +that reproduce anywhere: + +| Column | Compared? | Why | +|---|---|---| +| `N`, backend/checkpoint entries, ratio | yes | deterministic — identical on CI and a dev box | +| `backend bytes` | no | environment-sensitive (measured +4 bytes on CI) | +| `checkpoint bytes` | no | same class as `backend bytes` | +| `backend ms`, `checkpoint ms` | no | wall-clock; varies run to run | + +So a change that moves the read counts fails E2 instead of silently rewriting the recorded +result. When the move is legitimate, refresh the baseline deliberately and commit it: + +```bash +SH_E2_UPDATE_BASELINE=1 pnpm -C experiments test e2-reconstruction-cost +``` + +`SH_E2_RESULTS_DIR=` redirects the per-run copy elsewhere. + ## E5 live (real model — manual, end-to-end) Model + provider + credentials are runtime inputs; no secrets live in the repo. diff --git a/experiments/RESULTS.md b/experiments/RESULTS.md index ae7b140..af1a6d5 100644 --- a/experiments/RESULTS.md +++ b/experiments/RESULTS.md @@ -5,14 +5,21 @@ Synthetic sessions, each compacted once with a fixed kept tail. Metric = entries + bytes returned by `backend.read()` during reconstruction (the slice each loader rebuilds from). `*` ms columns are wall-clock on a dev box — **illustrative only**; the gate is the -deterministic entries ratio. +deterministic entries ratio. `backend bytes` is environment-sensitive too (it differs +between CI and a dev box), so the committed copy of this file is compared on the entries +and ratio columns only. + +This file is a **committed baseline**: `e2-reconstruction-cost.test.ts` asserts that a fresh +run still matches those columns, and writes each run's own table to the gitignored +`experiments/.results/`. Refresh it deliberately when a change legitimately moves the read +counts: `SH_E2_UPDATE_BASELINE=1 pnpm -C experiments test e2-reconstruction-cost`. | N (session len) | backend entries | checkpoint entries | ratio (b/c) | backend bytes | checkpoint bytes | backend ms* | checkpoint ms* | |---|---|---|---|---|---|---|---| -| 50 | 53 | 6 | 8.8 | 7482 | 896 | 0.9 | 1.0 | -| 200 | 203 | 6 | 33.8 | 28508 | 901 | 1.4 | 1.6 | -| 1000 | 1003 | 6 | 167.2 | 140908 | 901 | 5.7 | 5.9 | -| 5000 | 5003 | 6 | 833.8 | 706909 | 906 | 22.7 | 20.9 | +| 50 | 53 | 6 | 8.8 | 7482 | 896 | 0.9 | 1.2 | +| 200 | 203 | 6 | 33.8 | 28508 | 901 | 2.1 | 3.2 | +| 1000 | 1003 | 6 | 167.2 | 140908 | 901 | 4.8 | 5.1 | +| 5000 | 5003 | 6 | 833.8 | 706909 | 906 | 23.5 | 20.0 | **Pass:** checkpoint entries stay ~constant (bounded by the kept tail) while backend entries grow linearly with N, so the backend/checkpoint ratio strictly increases with N. `buildSessionContext()` diff --git a/experiments/src/report.ts b/experiments/src/report.ts index 8e5f5df..a894a55 100644 --- a/experiments/src/report.ts +++ b/experiments/src/report.ts @@ -9,6 +9,74 @@ export interface E2Row { checkpointMs: number; } +/** + * The subset of an E2 row that is reproducible on any machine, and therefore the only part + * a committed baseline can assert on. Measured across a dev box and CI: + * + * - entries + ratio: identical (synthetic fixtures, deterministic reads) + * - backendBytes: differs by +4 bytes in CI -- serialization is environment-sensitive + * - backendMs / checkpointMs: vary run to run even on one machine + * + * Ratio is rounded to the 1 decimal the markdown table carries, so a value read back from + * RESULTS.md compares equal to a freshly measured one. + */ +export interface E2Deterministic { + n: number; + backendEntries: number; + checkpointEntries: number; + ratioEntries: number; +} + +export function deterministicView(rows: E2Row[]): E2Deterministic[] { + return rows.map((r) => ({ + n: r.n, + backendEntries: r.backendEntries, + checkpointEntries: r.checkpointEntries, + ratioEntries: Number(r.ratioEntries.toFixed(1)), + })); +} + +/** + * Read the E2 table back out of a RESULTS.md. Used to compare a fresh run against the + * committed baseline; tolerates surrounding prose and unrelated tables by keying off the + * 8-column shape that buildResultsMarkdown emits. + */ +export function parseE2Table(markdown: string): E2Row[] { + const rows: E2Row[] = []; + for (const line of markdown.split("\n")) { + const trimmed = line.trim(); + if (!trimmed.startsWith("|")) continue; + const cells = trimmed.slice(1, trimmed.endsWith("|") ? -1 : undefined).split("|"); + if (cells.length !== 8) continue; // header, separator, and other tables + const nums = cells.map((c) => Number(c.trim())); + if (nums.some((v) => !Number.isFinite(v))) continue; // header/separator row + const [ + n, + backendEntries, + checkpointEntries, + ratioEntries, + backendBytes, + checkpointBytes, + backendMs, + checkpointMs, + ] = nums; + rows.push({ + n, + backendEntries, + checkpointEntries, + ratioEntries, + backendBytes, + checkpointBytes, + backendMs, + checkpointMs, + }); + } + if (rows.length === 0) { + throw new Error("no E2 table found: expected the 8-column table buildResultsMarkdown emits"); + } + return rows; +} + export function buildResultsMarkdown(rows: E2Row[]): string { const header = "| N (session len) | backend entries | checkpoint entries | ratio (b/c) | backend bytes | checkpoint bytes | backend ms* | checkpoint ms* |\n" + @@ -26,7 +94,14 @@ export function buildResultsMarkdown(rows: E2Row[]): string { Synthetic sessions, each compacted once with a fixed kept tail. Metric = entries + bytes returned by \`backend.read()\` during reconstruction (the slice each loader rebuilds from). \`*\` ms columns are wall-clock on a dev box — **illustrative only**; the gate is the -deterministic entries ratio. +deterministic entries ratio. \`backend bytes\` is environment-sensitive too (it differs +between CI and a dev box), so the committed copy of this file is compared on the entries +and ratio columns only. + +This file is a **committed baseline**: \`e2-reconstruction-cost.test.ts\` asserts that a fresh +run still matches those columns, and writes each run's own table to the gitignored +\`experiments/.results/\`. Refresh it deliberately when a change legitimately moves the read +counts: \`SH_E2_UPDATE_BASELINE=1 pnpm -C experiments test e2-reconstruction-cost\`. ${header} ${body} diff --git a/experiments/test/e2-reconstruction-cost.test.ts b/experiments/test/e2-reconstruction-cost.test.ts index 7b92923..bf4ce28 100644 --- a/experiments/test/e2-reconstruction-cost.test.ts +++ b/experiments/test/e2-reconstruction-cost.test.ts @@ -1,12 +1,18 @@ import { describe, it, expect, afterAll } from "vitest"; -import { writeFileSync } from "node:fs"; +import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import { SessionManager, type FileEntry } from "@earendil-works/pi-coding-agent"; import { RedisSessionBackend } from "@sh/session-backend"; import { BufferedRedisBackend } from "@sh/harness/buffered-redis-backend"; import { CountingBackend } from "../src/counting-backend"; import { buildCompactedSession } from "../src/session-fixture"; -import { buildResultsMarkdown, type E2Row } from "../src/report"; +import { + buildResultsMarkdown, + deterministicView, + parseE2Table, + type E2Row, +} from "../src/report"; const REDIS = process.env.REDIS_URL ?? "redis://127.0.0.1:6379"; const store = new RedisSessionBackend(REDIS); @@ -82,9 +88,30 @@ describe("E2 — reconstruction cost", () => { // And the largest N dwarfs the smallest. expect(rows[rows.length - 1].ratioEntries).toBeGreaterThan(rows[0].ratioEntries * 5); - // Record results next to this file. - const resultsPath = fileURLToPath(new URL("../RESULTS.md", import.meta.url)); - writeFileSync(resultsPath, buildResultsMarkdown(rows)); + const report = buildResultsMarkdown(rows); + + // Fresh measurements go to a gitignored path. This used to overwrite the committed + // RESULTS.md on every run, so any `pnpm -r test` left the working tree dirty with + // machine-local wall-clock timings. Override the directory with SH_E2_RESULTS_DIR. + const outDir = process.env.SH_E2_RESULTS_DIR + ? resolve(process.env.SH_E2_RESULTS_DIR) + : fileURLToPath(new URL("../.results", import.meta.url)); + mkdirSync(outDir, { recursive: true }); + writeFileSync(join(outDir, "RESULTS.md"), report); + + // The committed RESULTS.md is a checked-in baseline: assert the reproducible columns + // still match it, so a change that moves the read counts has to be acknowledged rather + // than quietly rewriting the recorded result. Only entries + ratio are compared -- + // backendBytes is environment-sensitive (+4 in CI) and the ms columns vary per run. + // Refresh deliberately with SH_E2_UPDATE_BASELINE=1 when a change legitimately moves them. + const baselinePath = fileURLToPath(new URL("../RESULTS.md", import.meta.url)); + if (process.env.SH_E2_UPDATE_BASELINE === "1") { + writeFileSync(baselinePath, report); + } else { + const baseline = parseE2Table(readFileSync(baselinePath, "utf8")); + expect(deterministicView(rows)).toEqual(deterministicView(baseline)); + } + // Echo to stdout (redirected to $LOG_DIR by the runner) for the record. console.log(JSON.stringify(rows, null, 2)); }, 120_000); diff --git a/experiments/test/report.test.ts b/experiments/test/report.test.ts new file mode 100644 index 0000000..6f44912 --- /dev/null +++ b/experiments/test/report.test.ts @@ -0,0 +1,99 @@ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { buildResultsMarkdown, parseE2Table, deterministicView, type E2Row } from "../src/report"; + +const ROWS: E2Row[] = [ + { + n: 50, + backendEntries: 53, + checkpointEntries: 6, + backendBytes: 7482, + checkpointBytes: 896, + ratioEntries: 53 / 6, + backendMs: 0.9, + checkpointMs: 1.0, + }, + { + n: 5000, + backendEntries: 5003, + checkpointEntries: 6, + backendBytes: 706909, + checkpointBytes: 906, + ratioEntries: 5003 / 6, + backendMs: 22.7, + checkpointMs: 20.9, + }, +]; + +describe("parseE2Table", () => { + it("round-trips the table that buildResultsMarkdown emits", () => { + const parsed = parseE2Table(buildResultsMarkdown(ROWS)); + expect(parsed).toHaveLength(ROWS.length); + expect(parsed.map((r) => r.n)).toEqual([50, 5000]); + expect(parsed.map((r) => r.backendEntries)).toEqual([53, 5003]); + expect(parsed.map((r) => r.checkpointEntries)).toEqual([6, 6]); + expect(parsed.map((r) => r.backendBytes)).toEqual([7482, 706909]); + expect(parsed.map((r) => r.checkpointBytes)).toEqual([896, 906]); + // The markdown carries ratio/ms at 1 decimal, so those come back rounded. + expect(parsed.map((r) => r.ratioEntries)).toEqual([8.8, 833.8]); + }); + + it("ignores prose and other tables around the E2 table", () => { + const md = `# Notes + +| unrelated | table | +|---|---| +| a | b | + +${buildResultsMarkdown(ROWS)}`; + expect(parseE2Table(md).map((r) => r.n)).toEqual([50, 5000]); + }); + + it("throws on a table with no data rows rather than returning nothing", () => { + expect(() => parseE2Table("# Empty\n\nno table here\n")).toThrow(/no E2 table/i); + }); +}); + +describe("deterministicView", () => { + it("keeps only the environment-independent columns", () => { + // backendBytes differs between CI and a dev box (+4 bytes, measured), and the ms + // columns vary run to run -- so neither can be part of a baseline comparison. + expect(deterministicView(ROWS)).toEqual([ + { n: 50, backendEntries: 53, checkpointEntries: 6, ratioEntries: 8.8 }, + { n: 5000, backendEntries: 5003, checkpointEntries: 6, ratioEntries: 833.8 }, + ]); + }); + + it("is stable across a build/parse round-trip, so a fresh run is comparable", () => { + const reparsed = parseE2Table(buildResultsMarkdown(ROWS)); + expect(deterministicView(reparsed)).toEqual(deterministicView(ROWS)); + }); + + it("is insensitive to byte and timing drift", () => { + const drifted = ROWS.map((r) => ({ + ...r, + backendBytes: r.backendBytes + 4, // the CI/local delta + backendMs: r.backendMs * 3, + checkpointMs: r.checkpointMs * 3, + })); + expect(deterministicView(drifted)).toEqual(deterministicView(ROWS)); + }); + + it("does notice a real change in the entries counts", () => { + const regressed = ROWS.map((r) => ({ ...r, checkpointEntries: r.checkpointEntries + 1 })); + expect(deterministicView(regressed)).not.toEqual(deterministicView(ROWS)); + }); +}); + +describe("the committed RESULTS.md baseline", () => { + it("parses, and its deterministic view survives a round-trip", () => { + const md = readFileSync(fileURLToPath(new URL("../RESULTS.md", import.meta.url)), "utf8"); + const baseline = parseE2Table(md); + expect(baseline.length).toBeGreaterThan(0); + // Guards against a hand-edit that breaks the table shape the E2 gate reads. + expect(deterministicView(parseE2Table(buildResultsMarkdown(baseline)))).toEqual( + deterministicView(baseline), + ); + }); +});