diff --git a/.changeset/eager-deer-hop.md b/.changeset/eager-deer-hop.md new file mode 100644 index 000000000..0bd08b65f --- /dev/null +++ b/.changeset/eager-deer-hop.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 4502 +--- +**A new offline benchmark reports the token savings from compact-content splits** — `npm run benchmark:compact-content` measures, per registered `workflow.compact_content` spine/detail split, the token count with and without the split active using a pinned tokenizer, and prints the reduction against a committed baseline without ever failing CI. (#4404) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index f6991909d..ea5fc7dfa 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -517,7 +517,7 @@ All workflow toggles follow the **absent = enabled** pattern. If a key is missin | `workflow.text_mode` | boolean | `false` | Replaces AskUserQuestion TUI menus with plain-text numbered lists. Required for Claude Code remote sessions (`/rc` mode) where TUI menus don't render. Can also be set per-session with `--text` flag on discuss-phase. Added in v1.28 | | `workflow.use_worktrees` | boolean | `true` | When `false`, disables git worktree isolation for parallel execution. Users who prefer sequential execution or whose environment does not support worktrees can disable this. Added in v1.31. **Branch-divergence note:** when your branch has diverged from `origin/HEAD`, GSD auto-degrades to sequential and prints a warning. See [`worktree.baseRef`](#worktree-settings) to restore parallel execution on a diverged branch. **Per-runtime note:** whether this key can be honored depends on the runtime's declared `dispatch.isolation` capability, not on its name (#2584). Runtimes whose own harness isolates each executor (**Claude Code**, **Cursor**) run parallel worktrees natively; runtimes exposing a headless exec with an explicit working directory (**Codex**, **OpenCode**, **Kimi**, **Kimi Code**) get worktrees GSD itself creates and merges — where a dispatch site can only drive the harness model, those hosts degrade to sequential with a warning rather than aborting. Every other runtime declares no isolation primitive, and forcing `use_worktrees: true` there still fails closed before any executor dispatch. `/gsd-health` reports such a value as warning `W025` (#2486). **Default on a non-Claude install:** if a worktree-capable non-Claude host is not isolating as described above, check whether the install stamped this key's default to `false` and set an explicit `use_worktrees: true`. See [Executor isolation per runtime](#executor-isolation-per-runtime). | | `workflow.agent_hint_routing` | boolean | `true` | Per-plan specialist executor routing (#1689). When `true`, a plan whose `agent_hint:` frontmatter names a subagent that resolves on the active runtime is dispatched to that specialist instead of `gsd-executor`. Default `true` — a no-op for plans without `agent_hint:`, so existing dispatch is unchanged. Set `false` to disable. See [PLAN.md `agent_hint`](reference/plan-md.md#per-plan-executor-routing). | -| `workflow.compact_content` | boolean | `false` | Compact content mode (#4139, [ADR-4139](adr/4139-compact-content-seam.md)). Per-project boolean selecting the terser form of GSD's own shipped prompt content (workflows, templates, agent-skill payloads). `/gsd-plan-phase` is the pilot workflow that actually branches on it today (#4402) — with the key off, its spine reads a deferred elaboration file back in before continuing (byte-identical instruction set to before); with it on, that read is skipped. The rest of the corpus does not branch on it yet — full coverage is later sub-issues (#4405+). | +| `workflow.compact_content` | boolean | `false` | Compact content mode (#4139, [ADR-4139](adr/4139-compact-content-seam.md)). Per-project boolean selecting the terser form of GSD's own shipped prompt content (workflows, templates, agent-skill payloads). `/gsd-plan-phase` is the pilot workflow that actually branches on it today (#4402) — with the key off, its spine reads a deferred elaboration file back in before continuing (byte-identical instruction set to before); with it on, that read is skipped. The rest of the corpus does not branch on it yet — full coverage is later sub-issues (#4405+). The eager-window token reduction each split actually achieves is measured, not asserted: `npm run benchmark:compact-content` reports per-split and aggregate on/off token counts (a proxy-tokenizer delta — Anthropic publishes no tokenizer for Claude 3+, so the comparison is exact under a pinned tokenizer even though the absolute counts are not Claude's real ones) against a committed baseline (`tests/fixtures/compact-content-benchmark-baseline.json`, #4404). Reporting-only — it never fails CI. | | `workflow.worktree_skip_hooks` | boolean | `false` | When `true`, executor agents in worktree mode pass `--no-verify` (skipping pre-commit hooks) and post-wave hook validation runs against the merged result instead. Opt-in escape hatch for projects whose hooks cannot run in agent worktrees. Default `false` runs hooks on every commit (#2924). | | `workflow.code_review` | boolean | `true` | Enable `/gsd-code-review` and `/gsd-code-review --fix` commands. When `false`, the commands exit with a configuration gate message. Added in v1.34 | | `workflow.code_review_point` | string | `execute:post` | Loop point at which the code-review capability's step registers: `execute:post` reviews once, after every wave in a phase has landed (default — unchanged behavior); `execute:wave:post` reviews once per completed wave instead, scoped to what changed since the phase's prior review (the whole phase's diff on the first wave, each subsequent wave's own diff thereafter). Manual `/gsd-code-review ` invocation is unaffected by this key — it is gated by `workflow.code_review` alone and runs regardless of which point is configured. `/gsd-autonomous` and `/gsd-quick` have no wave granularity of their own, so setting this to `execute:wave:post` means code review does not run automatically inside those two flows (consistent with how every other `execute:wave:post`-only capability already behaves for them). Added in #3661 | diff --git a/package-lock.json b/package-lock.json index 0a611180a..fce181925 100644 --- a/package-lock.json +++ b/package-lock.json @@ -30,6 +30,7 @@ "espree": "^10.4.0", "fast-check": "^4.8.0", "globals": "^16.5.0", + "gpt-tokenizer": "4.0.0", "js-yaml": "^4.3.1", "mutation-testing-metrics": "^3.7.3", "re2js": "^2.8.6", @@ -3565,6 +3566,13 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/gpt-tokenizer": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/gpt-tokenizer/-/gpt-tokenizer-4.0.0.tgz", + "integrity": "sha512-YAWIyzvuVUHEfW7tFfFAxH8qQb+Q3RU9nYOTy7skMNX5qzU6Q8jxTHZLyO56ug1vYvCR7wndzpd3jwD86/mhjQ==", + "dev": true, + "license": "MIT" + }, "node_modules/graceful-fs": { "version": "4.2.11", "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.2.11.tgz", diff --git a/package.json b/package.json index f04770584..677557620 100644 --- a/package.json +++ b/package.json @@ -75,6 +75,7 @@ "espree": "^10.4.0", "fast-check": "^4.8.0", "globals": "^16.5.0", + "gpt-tokenizer": "4.0.0", "js-yaml": "^4.3.1", "mutation-testing-metrics": "^3.7.3", "re2js": "^2.8.6", @@ -138,6 +139,7 @@ "lint:docs-command-form": "node scripts/lint-docs-command-form.cjs", "lint:hooks-runtime-build-seam": "node scripts/lint-hooks-runtime-build-seam.cjs", "ci:test-scope": "node scripts/ci-test-scope.cjs", + "benchmark:compact-content": "node scripts/benchmark-compact-content.cjs --check", "changeset": "node scripts/changeset/new.cjs", "changelog:render": "node scripts/changeset/cli.cjs render", "test": "node scripts/run-tests.cjs", diff --git a/scripts/benchmark-compact-content.cjs b/scripts/benchmark-compact-content.cjs new file mode 100644 index 000000000..4c8bc0abd --- /dev/null +++ b/scripts/benchmark-compact-content.cjs @@ -0,0 +1,368 @@ +#!/usr/bin/env node +'use strict'; + +/** + * Benchmarks the token-count effect of every registered `workflow.compact_content` + * spine/detail split (ADR-4139 Decision 3, Phase 3 #4403's `docs/PARTITION-RULES.md`). + * + * For every split it reports the "off" token count (spine + all detail parts — + * what `workflow.compact_content=false` pays today, since an opted-out project's + * spine reads every detail part back in) against the "on" token count (spine + * alone — what `workflow.compact_content=true` pays, since the detail `Read` + * never fires). See `.gsd/phase/enhance-4404-token-benchmark/40-design.md` for + * the full design rationale. + * + * PROXY-TOKENIZER CAVEAT: Anthropic publishes no tokenizer for Claude 3+, so + * `gpt-tokenizer` (pinned exact-version devDependency, see ADR-4139 + * Consequences) is a stand-in. The on/off COMPARISON is exact under this one + * pinned tokenizer applied identically to both sides; the absolute counts are + * NOT Claude's real token counts. Every output surface says so explicitly — + * see the `label` field below. + * + * Discovery is REIMPLEMENTED here rather than imported from + * `tests/helpers/compact-content-split.cjs`: a `scripts/` reporting tool + * depending on a test-only helper module inverts this repo's normal layering, + * and a test-only module changing shape should never be able to break a + * benchmark. The discovery logic is intentionally small (see + * `discoverRegisteredSplits` below) and mirrors `docs/PARTITION-RULES.md`'s + * own description of what a "registered split" is. + * + * Usage: + * node scripts/benchmark-compact-content.cjs # print JSON to stdout + * node scripts/benchmark-compact-content.cjs --write # write the committed baseline + * node scripts/benchmark-compact-content.cjs --check # recompute, diff vs committed baseline, print drift + * node scripts/benchmark-compact-content.cjs --check --baseline-path= + * # --check against a DIFFERENT baseline + * # file (testability seam; defaults to the + * # committed path when omitted) + * + * ============================================================================ + * CRITICAL — READ BEFORE "FIXING" ANYTHING BELOW (this is the issue's own + * Done-when item; getting it wrong defeats the entire point of this tool): + * + * This script — and in particular `--check` — MUST NEVER exit non-zero + * because a baseline is drifted, stale, or missing entirely. A drifted or + * absent baseline is REPORTED (printed to stdout as a human-readable diff), + * never treated as failure. The ONLY thing allowed to make this script exit + * non-zero is a genuine I/O error reading a SOURCE file the benchmark is + * measuring (a spine or detail `.md` file that `discoverRegisteredSplits` + * found on disk but that fails to read) — never anything about the baseline + * file. This is deliberate: the benchmark is a reporting instrument wired + * into `npm run benchmark:compact-content`, which is never part of a gate, + * `lint:ci`, or `pretest` — see `tests/benchmark-compact-content.test.cjs`'s + * "never-fails-CI" tests, which assert this behavior directly against a + * deliberately-wrong and a wholly-missing baseline. Do not add + * `process.exit(1)` (or an `ExitError` with a non-zero code) on drift. + * ============================================================================ + */ + +const fs = require('node:fs'); +const path = require('node:path'); + +const { countTokens } = require('gpt-tokenizer'); +const { runMain } = require('./lib/cli-exit.cjs'); + +const ROOT = path.resolve(__dirname, '..'); +const WORKFLOWS_DIR = path.join(ROOT, 'gsd-core', 'workflows'); +const BASELINE_PATH = path.join(ROOT, 'tests', 'fixtures', 'compact-content-benchmark-baseline.json'); + +// The tokenizer's own package.json is read live (`require.resolve` + a plain +// `fs.readFileSync`/JSON.parse — NOT `require('gpt-tokenizer/package.json')`, +// which would work identically here but would tie this file to Node's CJS +// JSON-import behavior for no benefit) rather than hardcoding the version as +// a string literal. This repo's own `package.json` pins `gpt-tokenizer` at an +// EXACT version (no `^`/`~`), so the two are guaranteed to agree today — but +// reading the installed package's own version live means a future re-pin of +// that dependency never requires touching this file too, and the reported +// version can never silently drift from what actually ran. +function getTokenizerVersion() { + const pkgPath = require.resolve('gpt-tokenizer/package.json'); + const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')); + return pkg.version; +} + +/** + * Discover every registered spine/detail split under `workflowsDir`. + * + * A split is registered by a `//detail/` directory + * (containing at least one `*.md` file) where `` is exactly ONE path + * segment directly under `workflowsDir`. Pairs with the spine at + * `/.md`; skipped entirely if that spine does not exist + * (this benchmark only measures real, complete splits — an orphaned detail + * directory with no spine is Phase 3's registration guard's problem, not + * this benchmark's). + * + * Mirrors `tests/helpers/compact-content-split.cjs`'s `discoverRegisteredSplits` + * in shape (same "one segment below workflowsDir, `detail/` dir, at least one + * `.md` file" rule) but is a from-scratch, self-contained implementation — + * see the module header for why this is not a shared import. + * + * @param {string} [workflowsDir] + * @returns {Array<{name: string, spinePath: string, detailPaths: string[]}>} + */ +function discoverRegisteredSplits(workflowsDir = WORKFLOWS_DIR) { + const found = new Map(); + + function walk(dir) { + let entries; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return; // unreadable directory: nothing to discover under it + } + for (const entry of entries) { + if (!entry.isDirectory()) continue; + const full = path.join(dir, entry.name); + + if (entry.name === 'detail') { + let mdFiles = []; + try { + mdFiles = fs + .readdirSync(full, { withFileTypes: true }) + .filter((e) => e.isFile() && e.name.endsWith('.md')) + .map((e) => e.name); + } catch { + mdFiles = []; + } + if (mdFiles.length > 0) { + const segments = path.relative(workflowsDir, dir).split(path.sep).filter(Boolean); + if (segments.length === 1) { + const name = segments[0]; + if (!found.has(name)) { + const spinePath = path.join(workflowsDir, `${name}.md`); + if (fs.existsSync(spinePath)) { + const detailPaths = mdFiles.map((f) => path.join(full, f)).sort(); + found.set(name, { name, spinePath, detailPaths }); + } + } + } + } + } + + walk(full); + } + } + + walk(workflowsDir); + return [...found.values()].sort((a, b) => a.name.localeCompare(b.name)); +} + +/** + * Compute the off/on/reduction numbers for ONE registered split. + * + * Throws on a genuine read failure of a source file (see the module-header + * CRITICAL note — this is the one thing allowed to throw). `reductionPct` is + * reported as `0` rather than `NaN`/`Infinity` when `offTokens` is `0` (an + * empty spine — should never happen for a real split, but must not crash). + * + * @param {{name: string, spinePath: string, detailPaths: string[]}} split + * @returns {{offTokens: number, onTokens: number, reductionPct: number}} + */ +function computeSplitTokens(split) { + const spineContent = fs.readFileSync(split.spinePath, 'utf8'); + const onTokens = countTokens(spineContent); + let detailTokens = 0; + for (const detailPath of split.detailPaths) { + const detailContent = fs.readFileSync(detailPath, 'utf8'); + detailTokens += countTokens(detailContent); + } + const offTokens = onTokens + detailTokens; + const reductionPct = offTokens === 0 ? 0 : round2(((offTokens - onTokens) / offTokens) * 100); + return { offTokens, onTokens, reductionPct }; +} + +function round2(n) { + return Math.round(n * 100) / 100; +} + +/** + * Aggregate per-split numbers. `aggregateReductionPct` is computed from the + * SUMMED off/on totals, never averaged across per-split percentages — a + * 2-split fixture with very different sizes is what + * `tests/benchmark-compact-content.test.cjs` uses to pin this down. Reports + * `0` (not `NaN`) when there are zero registered splits or `aggregateOff` is + * `0` — a valid, non-crashing state, not an error. + * + * @param {Record} splitResults + * @returns {{offTokens: number, onTokens: number, reductionPct: number}} + */ +function computeAggregate(splitResults) { + let offTokens = 0; + let onTokens = 0; + for (const key of Object.keys(splitResults)) { + offTokens += splitResults[key].offTokens; + onTokens += splitResults[key].onTokens; + } + const reductionPct = offTokens === 0 ? 0 : round2(((offTokens - onTokens) / offTokens) * 100); + return { offTokens, onTokens, reductionPct }; +} + +const LABEL = + "PROXY-TOKENIZER DELTA — gpt-tokenizer is a stand-in; Anthropic publishes no tokenizer for Claude 3+. " + + "The on/off COMPARISON is exact under this pinned tokenizer; absolute counts are not Claude's real token counts."; + +/** + * Build the full report object — the "committed baseline" shape. Keys of + * `splits` are sorted (`discoverRegisteredSplits` already returns + * name-sorted records; `Object.keys` insertion order on a plain object built + * in that order preserves it, and `--check`'s comparison re-sorts anyway so + * this is belt-and-suspenders, not load-bearing). + * + * @param {string} [workflowsDir] + * @returns {object} + */ +function buildReport(workflowsDir = WORKFLOWS_DIR) { + const splits = discoverRegisteredSplits(workflowsDir); + const splitReports = {}; + for (const split of splits) { + splitReports[split.name] = computeSplitTokens(split); + } + return { + schema_version: 1, + generated_by: 'scripts/benchmark-compact-content.cjs', + tokenizer: { name: 'gpt-tokenizer', version: getTokenizerVersion() }, + label: LABEL, + splits: splitReports, + aggregate: computeAggregate(splitReports), + }; +} + +/** + * Format a human-readable drift summary between a (possibly missing/invalid) + * committed baseline and a freshly-computed live report. Never throws — a + * missing or unparseable baseline is reported as "no baseline found" / + * "baseline could not be parsed", not propagated as an error, per the + * module's CRITICAL contract. + * + * @param {string} baselinePath + * @param {object} live + * @returns {string} + */ +function formatDriftReport(baselinePath, live) { + const lines = []; + let baseline = null; + let baselineReadError = null; + try { + const raw = fs.readFileSync(baselinePath, 'utf8'); + try { + baseline = JSON.parse(raw); + } catch (parseErr) { + baselineReadError = `baseline at ${baselinePath} could not be parsed as JSON: ${parseErr.message}`; + } + } catch { + baselineReadError = `no baseline found at ${baselinePath}`; + } + + if (baselineReadError) { + lines.push(`DRIFT: ${baselineReadError} — treating as fully drifted (this is reported, not an error).`); + lines.push('Live splits:'); + for (const name of Object.keys(live.splits).sort()) { + const s = live.splits[name]; + lines.push(` + ${name}: off=${s.offTokens} on=${s.onTokens} reduction=${s.reductionPct}%`); + } + lines.push( + `Live aggregate: off=${live.aggregate.offTokens} on=${live.aggregate.onTokens} ` + + `reduction=${live.aggregate.reductionPct}%`, + ); + return lines.join('\n'); + } + + const baselineSplits = (baseline && typeof baseline === 'object' && baseline.splits) || {}; + const liveSplits = live.splits; + const allNames = new Set([...Object.keys(baselineSplits), ...Object.keys(liveSplits)]); + let anyDrift = false; + + if (!baseline || typeof baseline.label !== 'string' || !baseline.label.includes('PROXY-TOKENIZER')) { + anyDrift = true; + lines.push('DRIFT: committed baseline is missing the required "PROXY-TOKENIZER" label.'); + } + + for (const name of [...allNames].sort()) { + const b = baselineSplits[name]; + const l = liveSplits[name]; + if (!b) { + anyDrift = true; + lines.push(`DRIFT: split "${name}" is new (not in committed baseline) — live off=${l.offTokens} on=${l.onTokens} reduction=${l.reductionPct}%`); + } else if (!l) { + anyDrift = true; + lines.push(`DRIFT: split "${name}" was removed (present in committed baseline, not found live) — baseline off=${b.offTokens} on=${b.onTokens} reduction=${b.reductionPct}%`); + } else if (b.offTokens !== l.offTokens || b.onTokens !== l.onTokens || b.reductionPct !== l.reductionPct) { + anyDrift = true; + lines.push( + `DRIFT: split "${name}": off ${b.offTokens} -> ${l.offTokens} (${l.offTokens - b.offTokens >= 0 ? '+' : ''}${l.offTokens - b.offTokens}), ` + + `on ${b.onTokens} -> ${l.onTokens} (${l.onTokens - b.onTokens >= 0 ? '+' : ''}${l.onTokens - b.onTokens}), ` + + `reduction ${b.reductionPct}% -> ${l.reductionPct}% (${round2(l.reductionPct - b.reductionPct) >= 0 ? '+' : ''}${round2(l.reductionPct - b.reductionPct)}pp)`, + ); + } + } + + const ba = (baseline && baseline.aggregate) || {}; + const la = live.aggregate; + if (ba.offTokens !== la.offTokens || ba.onTokens !== la.onTokens || ba.reductionPct !== la.reductionPct) { + anyDrift = true; + lines.push( + `DRIFT: aggregate: off ${ba.offTokens} -> ${la.offTokens}, on ${ba.onTokens} -> ${la.onTokens}, ` + + `reduction ${ba.reductionPct}% -> ${la.reductionPct}%`, + ); + } + + if (!anyDrift) { + lines.push(`Baseline at ${baselinePath} is up to date with the live recompute.`); + } else { + lines.push(''); + lines.push('Run `node scripts/benchmark-compact-content.cjs --write` to refresh the committed baseline.'); + lines.push('(This is a REPORT, not a gate — exiting 0 regardless of drift, per this script\'s own contract.)'); + } + return lines.join('\n'); +} + +function parseArgs(argv) { + const opts = { write: false, check: false, baselinePath: BASELINE_PATH }; + for (const arg of argv) { + if (arg === '--write') opts.write = true; + else if (arg === '--check') opts.check = true; + else if (arg.startsWith('--baseline-path=')) opts.baselinePath = arg.slice('--baseline-path='.length); + } + return opts; +} + +function main() { + const opts = parseArgs(process.argv.slice(2)); + + if (opts.write) { + const report = buildReport(); + fs.mkdirSync(path.dirname(BASELINE_PATH), { recursive: true }); + fs.writeFileSync(BASELINE_PATH, JSON.stringify(report, null, 2) + '\n'); + process.stdout.write(`Wrote ${BASELINE_PATH}\n`); + return; + } + + if (opts.check) { + const live = buildReport(); + // See the module-header CRITICAL note: this branch NEVER throws or sets a + // non-zero exit code on a drifted/missing/invalid baseline — only + // `buildReport()` above (a genuine source-file read failure) can throw. + process.stdout.write(formatDriftReport(opts.baselinePath, live) + '\n'); + return; + } + + process.stdout.write(JSON.stringify(buildReport(), null, 2) + '\n'); +} + +/* c8 ignore next 3 -- CLI entry guard; this repo measures coverage with c8, which does not honor istanbul pragmas */ +if (require.main === module) { + runMain(main); +} + +module.exports = { + discoverRegisteredSplits, + computeSplitTokens, + computeAggregate, + buildReport, + formatDriftReport, + getTokenizerVersion, + parseArgs, + LABEL, + BASELINE_PATH, + WORKFLOWS_DIR, +}; diff --git a/tests/benchmark-compact-content.test.cjs b/tests/benchmark-compact-content.test.cjs new file mode 100644 index 000000000..ab2bfc29e --- /dev/null +++ b/tests/benchmark-compact-content.test.cjs @@ -0,0 +1,446 @@ +'use strict'; + +/** + * Tests for scripts/benchmark-compact-content.cjs (Phase 4, #4404). Follows + * `.gsd/phase/enhance-4404-token-benchmark/50-test-matrix.md` row by row. + * + * 80/20 risk zone per that matrix: the "never fails CI" property and the + * offline/determinism proof are the highest-value tests here — a benchmark + * that accidentally starts gating CI, or that silently drifts + * non-deterministically, defeats the whole point of building a reporting + * instrument. The token-count arithmetic itself is exercised mostly via + * synthetic fixtures/direct function calls (fast, no subprocess needed). + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); + +const { createTempDir, cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); +const fc = require('./helpers/fast-check-setup.cjs'); +const { discoverRegisteredSplits: discoverRegisteredSplitsViaSharedHelper } = require('./helpers/compact-content-split.cjs'); + +const ROOT = path.resolve(__dirname, '..'); +const SCRIPT = path.join(ROOT, 'scripts', 'benchmark-compact-content.cjs'); +const BASELINE_PATH = path.join(ROOT, 'tests', 'fixtures', 'compact-content-benchmark-baseline.json'); +const DENY_NETWORK_PRELOAD = path.join(__dirname, 'fixtures', 'deny-network.cjs'); + +const benchmark = require('../scripts/benchmark-compact-content.cjs'); + +function runBenchmark(args = [], options = {}) { + return runNode([SCRIPT, ...args], { cwd: ROOT, timeoutMs: PROBE_TIMEOUT_MS, ...options }); +} + +// ─── Discovery ────────────────────────────────────────────────────────────── + +describe('discovery', () => { + test('the real plan-phase split is found, with sane on/off counts', () => { + const splits = benchmark.discoverRegisteredSplits(); + const planPhase = splits.find((s) => s.name === 'plan-phase'); + assert.ok(planPhase, 'plan-phase must be discovered from the real repo tree'); + const result = benchmark.computeSplitTokens(planPhase); + assert.ok(result.onTokens > 0); + assert.ok(result.offTokens > result.onTokens, 'off must be strictly larger than on for a real split with content'); + assert.ok(result.reductionPct > 0 && result.reductionPct < 100); + }); + + test('a split with an empty detail file: off equals on, 0% reduction, no NaN/Infinity', () => { + const tmp = createTempDir('gsd-bench-discovery-'); + try { + const workflowsDir = path.join(tmp, 'workflows'); + fs.mkdirSync(path.join(workflowsDir, 'only', 'detail'), { recursive: true }); + fs.writeFileSync(path.join(workflowsDir, 'only.md'), '# Spine\n\nSome real content here.\n'); + fs.writeFileSync(path.join(workflowsDir, 'only', 'detail', 'empty.md'), ''); + + const splits = benchmark.discoverRegisteredSplits(workflowsDir); + assert.strictEqual(splits.length, 1); + const result = benchmark.computeSplitTokens(splits[0]); + assert.strictEqual(result.offTokens, result.onTokens); + assert.strictEqual(result.reductionPct, 0); + assert.ok(Number.isFinite(result.reductionPct)); + } finally { + cleanup(tmp); + } + }); + + test('zero registered splits at all: discovery returns [], aggregate reports an explicit 0/0 state', () => { + const tmp = createTempDir('gsd-bench-discovery-empty-'); + try { + const workflowsDir = path.join(tmp, 'workflows'); + fs.mkdirSync(workflowsDir, { recursive: true }); + fs.writeFileSync(path.join(workflowsDir, 'lonely.md'), '# No detail dir for this one\n'); + + const splits = benchmark.discoverRegisteredSplits(workflowsDir); + assert.deepStrictEqual(splits, []); + const aggregate = benchmark.computeAggregate({}); + assert.deepStrictEqual(aggregate, { offTokens: 0, onTokens: 0, reductionPct: 0 }); + } finally { + cleanup(tmp); + } + }); + + test('two discovery calls against the same real tree agree, order-independent', () => { + const a = benchmark.discoverRegisteredSplits().map((s) => s.name).sort(); + const b = benchmark.discoverRegisteredSplits().map((s) => s.name).sort(); + assert.deepStrictEqual(a, b); + }); + + test('parity: this script\'s own discovery agrees with tests/helpers/compact-content-split.cjs on the real tree', () => { + // This module's header explains WHY discovery is reimplemented here rather + // than importing the shared test helper (a scripts/ reporting tool must + // not depend on a test-only module). CLAUDE.md's "Generative Fix + // Divergence" rule requires a parity assertion for exactly this shape — + // two independently-maintained copies of the same discovery rule that + // could silently drift apart. This test is that assertion: it does NOT + // import the shared helper into the script, it only proves the two + // implementations still agree on the real repo tree today. + const ownResults = benchmark.discoverRegisteredSplits(); + const sharedResults = discoverRegisteredSplitsViaSharedHelper(); + + const ownByName = new Map(ownResults.map((s) => [s.name, s])); + // The shared helper also reports a split whose spine file is missing + // (spineExists: false); this script's own discovery filters those out + // entirely (see the `fs.existsSync(spinePath)` guard above), so parity is + // scoped to splits BOTH implementations agree are real. + const sharedByName = new Map(sharedResults.filter((s) => s.spineExists).map((s) => [s.name, s])); + + assert.deepStrictEqual([...ownByName.keys()].sort(), [...sharedByName.keys()].sort()); + for (const name of ownByName.keys()) { + const own = ownByName.get(name); + const shared = sharedByName.get(name); + assert.strictEqual(own.spinePath, shared.spinePath, `spinePath diverged for split "${name}"`); + assert.deepStrictEqual(own.detailPaths, shared.detailPaths, `detailPaths diverged for split "${name}"`); + } + }); +}); + +// ─── Reduction math ───────────────────────────────────────────────────────── + +describe('reduction math', () => { + test('the arithmetic does not clamp or hide a negative percentage', () => { + // A synthetic input fed directly to computeAggregate (never producible by + // computeSplitTokens's own formula, where off = on + detail >= on always) — + // this exercises that the aggregate formula reports whatever the numbers + // say, rather than silently clamping a reduction below zero to zero. + const aggregate = benchmark.computeAggregate({ + weird: { offTokens: 100, onTokens: 150 }, + }); + assert.strictEqual(aggregate.offTokens, 100); + assert.strictEqual(aggregate.onTokens, 150); + assert.ok(aggregate.reductionPct < 0, 'a negative reduction must be reported as-is, not clamped to 0'); + }); + + test('aggregate is a real sum of two very differently-sized splits, not an average of percentages', () => { + // Split A: 90% reduction on a huge detail file. Split B: 10% reduction on a + // tiny one. An averaged-percentage bug would report (90+10)/2 = 50%; the + // real sum-of-tokens formula must instead be dominated by the larger split. + const splitResults = { + big: { offTokens: 10000, onTokens: 1000 }, // 90% reduction + small: { offTokens: 100, onTokens: 90 }, // 10% reduction + }; + const aggregate = benchmark.computeAggregate(splitResults); + assert.strictEqual(aggregate.offTokens, 10100); + assert.strictEqual(aggregate.onTokens, 1090); + const expectedPct = Math.round(((10100 - 1090) / 10100) * 100 * 100) / 100; + assert.strictEqual(aggregate.reductionPct, expectedPct); + assert.notStrictEqual(aggregate.reductionPct, 50, 'must not be the naive average of 90% and 10%'); + }); + + // CLAUDE.md's Property-Based Testing rule requires at least one fast-check + // property test for budget-limit arithmetic; computeAggregate's off/on + // token summation is exactly that. + test('property: computeAggregate is a true sum over any number of splits, never NaN/Infinity, and never exceeds the summed off total', () => { + fc.assert( + fc.property( + fc.dictionary( + fc.stringMatching(/^[a-z][a-z0-9-]{0,19}$/), + fc.record({ + onTokens: fc.nat({ max: 1_000_000 }), + extraDetailTokens: fc.nat({ max: 1_000_000 }), + }), + { minKeys: 0, maxKeys: 20 }, + ), + (splitInputs) => { + const splitResults = {}; + let expectedOff = 0; + let expectedOn = 0; + for (const key of Object.keys(splitInputs)) { + const { onTokens, extraDetailTokens } = splitInputs[key]; + const offTokens = onTokens + extraDetailTokens; // mirrors computeSplitTokens's own invariant: off >= on + splitResults[key] = { offTokens, onTokens }; + expectedOff += offTokens; + expectedOn += onTokens; + } + + const aggregate = benchmark.computeAggregate(splitResults); + + assert.strictEqual(aggregate.offTokens, expectedOff); + assert.strictEqual(aggregate.onTokens, expectedOn); + assert.ok(Number.isFinite(aggregate.reductionPct), 'reductionPct must never be NaN/Infinity'); + assert.ok(aggregate.reductionPct <= 100, 'reductionPct can never exceed 100% when off >= on for every split'); + }, + ), + ); + }); +}); + +// ─── Determinism ──────────────────────────────────────────────────────────── + +describe('determinism', () => { + test('two consecutive real runs (default stdout mode) are byte-identical', () => { + const r1 = runBenchmark(); + const r2 = runBenchmark(); + assert.strictEqual(r1.exitCode, 0, `stderr: ${r1.stderr}`); + assert.strictEqual(r2.exitCode, 0, `stderr: ${r2.stderr}`); + assert.strictEqual(r1.stdout, r2.stdout); + }); + + test('running from two different cwds produces the same result', () => { + const otherCwd = os.tmpdir(); + const r1 = runBenchmark([], { cwd: ROOT }); + const r2 = runBenchmark([], { cwd: otherCwd }); + assert.strictEqual(r1.exitCode, 0, `stderr: ${r1.stderr}`); + assert.strictEqual(r2.exitCode, 0, `stderr: ${r2.stderr}`); + assert.strictEqual(r1.stdout, r2.stdout); + }); + + test('a genuine read error on a discovered source file throws loud, never silently dropped from the aggregate', () => { + const realFs = require('node:fs'); + const originalReadFileSync = realFs.readFileSync; + const targetPath = path.join(ROOT, 'gsd-core', 'workflows', 'plan-phase.md'); + // Method-monkeypatching, not chmod: deterministic cross-platform IO-failure + // injection per this repo's own documented preference (chmod 000 is a + // no-op under root/CI and on Windows). Restored in `finally` regardless of + // assertion outcome, so no other test in this process observes the stub. + realFs.readFileSync = function stubbedReadFileSync(p, ...rest) { + if (p === targetPath) { + throw new Error('ENOENT-synthetic: simulated unreadable source file for this test'); + } + return originalReadFileSync.call(realFs, p, ...rest); + }; + try { + assert.throws(() => benchmark.buildReport(), /simulated unreadable source file/); + } finally { + realFs.readFileSync = originalReadFileSync; + } + // Prove the stub didn't leak: a normal call succeeds again afterward. + assert.doesNotThrow(() => benchmark.buildReport()); + }); +}); + +// ─── Offline proof ────────────────────────────────────────────────────────── + +describe('offline proof', () => { + test('the deny-network preload is not a no-op: a network-touching probe throws under it', () => { + const tmp = createTempDir('gsd-bench-offline-'); + try { + const probePath = path.join(tmp, 'dns-probe.cjs'); + fs.writeFileSync(probePath, "require('node:dns').lookup('example.com', () => {});\n"); + const result = runNode([probePath], { + cwd: ROOT, + timeoutMs: PROBE_TIMEOUT_MS, + env: { ...process.env, NODE_OPTIONS: `--require ${DENY_NETWORK_PRELOAD}` }, + }); + assert.notStrictEqual(result.exitCode, 0, 'a network-touching script must NOT exit cleanly under the deny-network preload'); + assert.match(result.stderr, /deny-network/); + } finally { + cleanup(tmp); + } + }); + + test('the benchmark itself runs to completion under the deny-network preload and emits valid JSON', () => { + const result = runBenchmark([], { + env: { ...process.env, NODE_OPTIONS: `--require ${DENY_NETWORK_PRELOAD}` }, + }); + assert.strictEqual(result.exitCode, 0, `stderr: ${result.stderr}`); + assert.doesNotThrow(() => JSON.parse(result.stdout)); + }); + + test('two consecutive runs under the deny-network preload are byte-identical (combined determinism + offline property)', () => { + // The "determinism" and "offline proof" describe-blocks above each prove + // one half of this on its own (two runs agree; one run survives with + // network denied). This test is the combined property the issue's + // Done-when criterion actually asks for: identical output ACROSS two + // runs THAT ARE BOTH network-denied, not each half verified separately. + const env = { ...process.env, NODE_OPTIONS: `--require ${DENY_NETWORK_PRELOAD}` }; + const r1 = runBenchmark([], { env }); + const r2 = runBenchmark([], { env }); + assert.strictEqual(r1.exitCode, 0, `stderr: ${r1.stderr}`); + assert.strictEqual(r2.exitCode, 0, `stderr: ${r2.stderr}`); + assert.strictEqual(r1.stdout, r2.stdout); + }); + + test('the preload does not leak into this (parent) test process', () => { + // The preload only ever runs inside the spawned child (NODE_OPTIONS is + // per-process); the parent's own http.request reference must be + // unaffected by the two child runs above. + const httpRequest = require('node:http').request; + assert.strictEqual(typeof httpRequest, 'function'); + assert.doesNotMatch(String(httpRequest), /deny-network/); + }); +}); + +// ─── Proxy-tokenizer labeling ─────────────────────────────────────────────── + +describe('proxy-tokenizer labeling', () => { + test('the committed baseline carries the PROXY-TOKENIZER label', () => { + const committed = JSON.parse(fs.readFileSync(BASELINE_PATH, 'utf8')); + assert.match(committed.label, /PROXY-TOKENIZER/); + }); + + test('a fresh run (subprocess, default stdout mode) carries the PROXY-TOKENIZER label', () => { + const result = runBenchmark(); + assert.strictEqual(result.exitCode, 0, `stderr: ${result.stderr}`); + const live = JSON.parse(result.stdout); + assert.match(live.label, /PROXY-TOKENIZER/); + }); + + test('--check flags a baseline that is missing the PROXY-TOKENIZER label', () => { + const live = benchmark.buildReport(); + const unlabeled = { ...live, label: 'a hand-edited pre-labeling-era baseline' }; + const output = benchmark.formatDriftReport('/nonexistent-path-not-used', unlabeled); + // formatDriftReport treats a read failure as "no baseline found"; to test + // the label check specifically we call it via a real temp file instead. + const tmp = createTempDir('gsd-bench-label-'); + try { + const baselinePath = path.join(tmp, 'baseline.json'); + fs.writeFileSync(baselinePath, JSON.stringify(unlabeled, null, 2)); + const report = benchmark.formatDriftReport(baselinePath, live); + assert.match(report, /DRIFT/); + assert.match(report, /PROXY-TOKENIZER/); + } finally { + cleanup(tmp); + } + assert.match(output, /DRIFT/); // the nonexistent-path branch also reports DRIFT + }); +}); + +// ─── Never-fails-CI ───────────────────────────────────────────────────────── + +describe('never-fails-CI', () => { + test('--check against the real, committed, non-drifted baseline exits 0', () => { + const result = runBenchmark(['--check']); + assert.strictEqual(result.exitCode, 0, `stderr: ${result.stderr}`); + assert.match(result.stdout, /up to date/); + }); + + test('--check against a baseline that differs by one token still exits 0, printing the diff', () => { + const tmp = createTempDir('gsd-bench-drift-'); + try { + const committed = JSON.parse(fs.readFileSync(BASELINE_PATH, 'utf8')); + const wrong = JSON.parse(JSON.stringify(committed)); + const firstSplit = Object.keys(wrong.splits)[0]; + wrong.splits[firstSplit].offTokens += 1; // deliberately wrong by one token + const wrongPath = path.join(tmp, 'wrong-baseline.json'); + fs.writeFileSync(wrongPath, JSON.stringify(wrong, null, 2)); + + const result = runBenchmark(['--check', `--baseline-path=${wrongPath}`]); + assert.strictEqual(result.exitCode, 0, `stderr: ${result.stderr}`); + assert.match(result.stdout, /DRIFT/); + assert.match(result.stdout, new RegExp(firstSplit)); + } finally { + cleanup(tmp); + } + }); + + test('--check against a wholly missing baseline file still exits 0, reported as fully drifted', () => { + const tmp = createTempDir('gsd-bench-missing-'); + try { + const missingPath = path.join(tmp, 'does-not-exist.json'); + const result = runBenchmark(['--check', `--baseline-path=${missingPath}`]); + assert.strictEqual(result.exitCode, 0, `stderr: ${result.stderr}`); + assert.match(result.stdout, /no baseline found/); + } finally { + cleanup(tmp); + } + }); + + test('running --check twice against the same drifted baseline reports the same drift both times (idempotent)', () => { + const tmp = createTempDir('gsd-bench-idempotent-'); + try { + const committed = JSON.parse(fs.readFileSync(BASELINE_PATH, 'utf8')); + const wrong = JSON.parse(JSON.stringify(committed)); + const firstSplit = Object.keys(wrong.splits)[0]; + wrong.splits[firstSplit].onTokens -= 1; + const wrongPath = path.join(tmp, 'wrong-baseline.json'); + fs.writeFileSync(wrongPath, JSON.stringify(wrong, null, 2)); + + const r1 = runBenchmark(['--check', `--baseline-path=${wrongPath}`]); + const r2 = runBenchmark(['--check', `--baseline-path=${wrongPath}`]); + assert.strictEqual(r1.exitCode, 0); + assert.strictEqual(r2.exitCode, 0); + assert.strictEqual(r1.stdout, r2.stdout); + } finally { + cleanup(tmp); + } + }); + + test('a genuine I/O error on a SOURCE file still throws (never masked by the never-fails-CI contract)', () => { + // Distinguishes "the baseline is never allowed to cause a failure" from + // "nothing can ever cause a failure" — the module-header CRITICAL comment + // in scripts/benchmark-compact-content.cjs makes exactly this distinction. + // Re-verified here via subprocess (module require cache in the discovery + // describe-block above already covers the in-process shape). + const tmp = createTempDir('gsd-bench-ioerror-'); + try { + const workflowsDir = path.join(tmp, 'workflows'); + fs.mkdirSync(path.join(workflowsDir, 'broken', 'detail'), { recursive: true }); + fs.writeFileSync(path.join(workflowsDir, 'broken.md'), '# spine\n'); + fs.writeFileSync(path.join(workflowsDir, 'broken', 'detail', 'part.md'), 'detail content\n'); + + // Directly exercise the exported function with a monkeypatched fs, + // rather than a real chmod: chmod 000 is a no-op under a root-run + // Docker/CI process and has no equivalent on Windows, so it is not a + // reliable cross-platform failure injector (see CLAUDE.md's IO-failure + // injection rule) — deterministic method-patching works everywhere. + const realFs = require('node:fs'); + const originalReadFileSync = realFs.readFileSync; + const targetPath = path.join(workflowsDir, 'broken', 'detail', 'part.md'); + realFs.readFileSync = function stubbedReadFileSync(p, ...rest) { + if (p === targetPath) throw new Error('EACCES-synthetic: permission denied'); + return originalReadFileSync.call(realFs, p, ...rest); + }; + try { + assert.throws(() => benchmark.buildReport(workflowsDir), /EACCES-synthetic/); + } finally { + realFs.readFileSync = originalReadFileSync; + } + } finally { + cleanup(tmp); + } + }); +}); + +// ─── gpt-tokenizer placement ──────────────────────────────────────────────── + +describe('gpt-tokenizer placement', () => { + const pkg = JSON.parse(fs.readFileSync(path.join(ROOT, 'package.json'), 'utf8')); + + test('devDependencies pins gpt-tokenizer at an exact (non-range) version', () => { + assert.ok(pkg.devDependencies && typeof pkg.devDependencies['gpt-tokenizer'] === 'string'); + const version = pkg.devDependencies['gpt-tokenizer']; + assert.doesNotMatch(version, /[\^~*x]/i, `expected an exact pin, got ${JSON.stringify(version)}`); + }); + + test('dependencies (production) does not carry gpt-tokenizer', () => { + assert.ok(!pkg.dependencies || !('gpt-tokenizer' in pkg.dependencies)); + }); + + test('package-lock.json resolves gpt-tokenizer to the exact pinned version', () => { + const lock = JSON.parse(fs.readFileSync(path.join(ROOT, 'package-lock.json'), 'utf8')); + const pinned = pkg.devDependencies['gpt-tokenizer']; + const resolved = lock.packages && lock.packages['node_modules/gpt-tokenizer']; + assert.ok(resolved, 'package-lock.json must carry a resolved entry for gpt-tokenizer'); + assert.strictEqual(resolved.version, pinned); + }); + + test('the reported tokenizer version matches the exact pinned devDependency', () => { + const report = benchmark.buildReport(); + assert.strictEqual(report.tokenizer.name, 'gpt-tokenizer'); + assert.strictEqual(report.tokenizer.version, pkg.devDependencies['gpt-tokenizer']); + }); +}); diff --git a/tests/fixtures/compact-content-benchmark-baseline.json b/tests/fixtures/compact-content-benchmark-baseline.json new file mode 100644 index 000000000..b0808ee18 --- /dev/null +++ b/tests/fixtures/compact-content-benchmark-baseline.json @@ -0,0 +1,21 @@ +{ + "schema_version": 1, + "generated_by": "scripts/benchmark-compact-content.cjs", + "tokenizer": { + "name": "gpt-tokenizer", + "version": "4.0.0" + }, + "label": "PROXY-TOKENIZER DELTA — gpt-tokenizer is a stand-in; Anthropic publishes no tokenizer for Claude 3+. The on/off COMPARISON is exact under this pinned tokenizer; absolute counts are not Claude's real token counts.", + "splits": { + "plan-phase": { + "offTokens": 27637, + "onTokens": 24347, + "reductionPct": 11.9 + } + }, + "aggregate": { + "offTokens": 27637, + "onTokens": 24347, + "reductionPct": 11.9 + } +} diff --git a/tests/fixtures/deny-network.cjs b/tests/fixtures/deny-network.cjs new file mode 100644 index 000000000..3b519dd31 --- /dev/null +++ b/tests/fixtures/deny-network.cjs @@ -0,0 +1,62 @@ +'use strict'; + +/** + * Network-denial preload for offline-proof tests (Phase 4, #4404). + * + * Loaded via `NODE_OPTIONS=--require ` when spawning a child + * process that must prove it made no network call. At module-load time this + * monkey-patches every network entry point Node exposes to a script running + * under the default CJS loader — `http`/`https` request/get, `net.Socket`'s + * `connect`, `dns` lookup/resolve variants (both the callback API and the + * separate `dns.promises` binding), `tls.connect`, `http2.connect`, and + * `global(This).fetch` (when defined) — with a function that throws + * synchronously, so ANY attempt to use one of these surfaces fails loudly + * and immediately rather than hanging or silently succeeding against a real + * or mocked network. + * + * No exports — this file is loaded purely for its side effects (per + * `NODE_OPTIONS=--require`'s contract, which does not consume a return + * value). The monkeypatches live for the lifetime of the process that loaded + * this preload; because that process is always a short-lived child spawned + * specifically for an offline-proof test, this never leaks into any other + * test's process. + */ + +function deny(name) { + return function denyNetworkCall() { + throw new Error(`deny-network: network access attempted during an offline-only test (${name})`); + }; +} + +const http = require('node:http'); +http.request = deny('http.request'); +http.get = deny('http.get'); + +const https = require('node:https'); +https.request = deny('https.request'); +https.get = deny('https.get'); + +const net = require('node:net'); +net.Socket.prototype.connect = deny('net.Socket.prototype.connect'); + +const dns = require('node:dns'); +dns.lookup = deny('dns.lookup'); +dns.resolve = deny('dns.resolve'); +dns.resolve4 = deny('dns.resolve4'); +dns.resolve6 = deny('dns.resolve6'); +// dns.promises is a separate binding — patching the callback functions above +// does not touch it. +dns.promises.lookup = deny('dns.promises.lookup'); +dns.promises.resolve = deny('dns.promises.resolve'); +dns.promises.resolve4 = deny('dns.promises.resolve4'); +dns.promises.resolve6 = deny('dns.promises.resolve6'); + +const tls = require('node:tls'); +tls.connect = deny('tls.connect'); + +const http2 = require('node:http2'); +http2.connect = deny('http2.connect'); + +if (typeof globalThis.fetch === 'function') { + globalThis.fetch = deny('fetch'); +}